Modify

Opened 9 months ago

Closed 7 months ago

#19381 closed enhancement (fixed)

Warn users about making changesets with unreasonably large bounding boxes

Reported by: mkoniecz Owned by: team
Priority: normal Milestone: 20.06
Component: Core Version:
Keywords: template_report upload changeset warning Cc:

Description (last modified by mkoniecz)

What steps will reproduce the problem?

  1. For start warning about +100km or +500km bounding box would be nice

What is the expected result?

https://www.openstreetmap.org/changeset/86588575#map=4/47.65/57.15 https://www.openstreetmap.org/changeset/86601765 - user will get warning

What happens instead?

No warning

Please provide any additional information below. Attach a screenshot if possible.

For bonus points - do not warn if objects being deleted or their original size was already massive (for example editing country relation will create enormous bbox and it is unavoidable)

For bonus points - warn when user starts editing far away from edits made so far

Partially inspired by feature request made, for some reason, on talk mailing list

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-06-12 19:48:35 +0200 (Fri, 12 Jun 2020)
Revision:16610
Build-Date:2020-06-13 01:30:48
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (16610 en_GB) Linux Ubuntu 19.10
Memory Usage: 947 MB / 976 MB (124 MB allocated, but free)
Java version: 11.0.7+10-post-Ubuntu-2ubuntu219.10, Ubuntu, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel
Screen: :0.0 1920x1080 (scaling 1.0x1.0)
Maximum Screen Size: 1920x1080
Best cursor sizes: 16x16 -> 16x16, 32x32 -> 32x32
Java package: openjdk-11-jre:amd64-11.0.7+10-2ubuntu2~19.10
Java ATK Wrapper package: libatk-wrapper-java:all-0.35.0-3
libcommons-logging-java: libcommons-logging-java:all-1.2-2
fonts-noto: fonts-noto:-
Dataset consistency test: No problems found

Plugins:
+ buildings_tools (35474)
+ measurement (35405)
+ reverter (35487)

Last errors/warnings:
- E: Region [TMS_BLOCK_v2] : Failure getting from disk--IOException, key = OpenStreetMap Carto (Standard):https://{switch:a,b,c}.tile.openstreetmap.org/{zoom}/{x}/{y}.png/14/9094/5549
- W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server

Attachments (7)

2020-06-16-181434.png (14.8 KB) - added by simon04 9 months ago.
2020-06-16-234254.png (19.8 KB) - added by simon04 9 months ago.
2020-06-16-234314.png (17.6 KB) - added by simon04 9 months ago.
2020-06-17-213853.png (16.3 KB) - added by simon04 8 months ago.
2020-06-17-230219.png (16.3 KB) - added by simon04 8 months ago.
19381flicker.gif (160.3 KB) - added by Klumbumbus 8 months ago.
19381cutoff.gif (963.3 KB) - added by Klumbumbus 8 months ago.

Download all attachments as: .zip

Change History (53)

comment:1 Changed 9 months ago by mkoniecz

Description: modified (diff)

comment:2 Changed 9 months ago by stoecker

Resolution: wontfix
Status: newclosed

If the server operators have no issue with large changesets there is no reason for us. There are many valid reasons to have large bounding boxes.

comment:3 Changed 9 months ago by skyper

I remember a situation where I unintentional uploaded a node hundreds kilometers away. I would have loved to get a warning.

The problem with big bbox is that you do not get much information especially if only some object here and some there were edited. Additionally it needs more resources when opening the changeset page on OSM.

comment:4 Changed 9 months ago by mkoniecz

I am not asking to block them completely, just add a warning.

Similarly there are valid reason for changeset without source (reverts) and with short description (sometimes single short word is sufficient) but JOSM complains about that.

"If the server operators have no issue with large changesets there is no reason for us."
I am not claiming that it is an operations issue (database/performance problem). I am claiming that edits with 500km+ bounding box are

Last edited 9 months ago by mkoniecz (previous) (diff)

comment:5 Changed 9 months ago by simon04

Keywords: upload changeset warning added
Resolution: wontfix
Status: closedreopened

I like the propsal. It nicely goes hand in hand with the already present checks "Please revise upload comment" and "Please specify a changeset source".

comment:6 Changed 9 months ago by mkoniecz

I was thinking about it a bit - and is there a chance to warn user proactively ("warn when user starts editing far away from edits made so far")?

Because unlike "Please revise upload comment" and "Please specify a changeset source" it seems to not be clearly fixable at time of making the upload.

Automatically splitting changeset would be likely problematic, and I am unsure is it possible to fix that problem in an obvious way - without redoing the work.

comment:7 in reply to:  6 Changed 9 months ago by skyper

Relation might be the tricky object type. Or should only be warned about new relations?

Replying to mkoniecz:

I was thinking about it a bit - and is there a chance to warn user proactively ("warn when user starts editing far away from edits made so far")?

This will need quite some calculations and probably not work in all case. Thinking about merging and loading files.

Because unlike "Please revise upload comment" and "Please specify a changeset source" it seems to not be clearly fixable at time of making the upload.

Right, but some help could be offered, see below.

Automatically splitting changeset would be likely problematic, and I am unsure is it possible to fix that problem in an obvious way - without redoing the work.

  1. Upload is possible using upload selection.
  2. Question is how to help the user
    • An option to set selection to problematic objects in MapView and upload dialog.
    • In the upload dialog problematic objects could get a colored background.
  3. It should be possible to deselect objects from upload in the upload dialog.

comment:8 Changed 9 months ago by stoecker

I'm still against this. JOSM recently gets more and more annoying checks for perfectly valid behaviour. These checks will cause opposite effects: Users will simply click away the checks instead of reading them.

comment:9 in reply to:  6 ; Changed 9 months ago by GerdP

Replying to mkoniecz:

I was thinking about it a bit - and is there a chance to warn user proactively ("warn when user starts editing far away from edits made so far")?

Because unlike "Please revise upload comment" and "Please specify a changeset source" it seems to not be clearly fixable at time of making the upload.

Automatically splitting changeset would be likely problematic, and I am unsure is it possible to fix that problem in an obvious way - without redoing the work.

You can "Upload selection".

comment:10 in reply to:  8 ; Changed 9 months ago by GerdP

Replying to stoecker:

I'm still against this. JOSM recently gets more and more annoying checks for perfectly valid behaviour. These checks will cause opposite effects: Users will simply click away the checks instead of reading them.

It would not help to just show a warning at upload time, but JOSM might be able to automatically split the uploaded changes so that bounding boxes don't cover more than x km².
Another possible user feedback would be to calculate the area covered by the currently changed objects and show it in the status line. That information could be displayed together with some kind of traffic light (green for small area, red for huge area).

comment:11 in reply to:  9 Changed 9 months ago by Klumbumbus

Replying to GerdP:

You can "Upload selection".

As soon as relations were changed this gets complicated.

comment:12 in reply to:  8 ; Changed 9 months ago by Klumbumbus

Replying to stoecker:

JOSM recently gets more and more annoying checks for perfectly valid behaviour.

What exactly do you mean?

comment:13 in reply to:  12 Changed 9 months ago by stoecker

Replying to Klumbumbus:

Replying to stoecker:

JOSM recently gets more and more annoying checks for perfectly valid behaviour.

What exactly do you mean?

The tendency that more and more warnings and notes and infos and ... are added for each and everything. Each of them seems sane in their own field, but altogether they start to be disturbing.

E.g. I use JOSM outside the OSM world for a project as data display tool and it took a considerable amount of time to silence many many warnings and notes cluttering and disturbing the display.

comment:14 in reply to:  10 Changed 9 months ago by stoecker

Replying to GerdP:

It would not help to just show a warning at upload time, but JOSM might be able to automatically split the uploaded changes so that bounding boxes don't cover more than x km².

Why? Large bounding boxes are not wrong. If the users don't think it's necessary to work in a small area only then this is perfectly fine.

comment:15 Changed 9 months ago by GerdP

Why? Large bounding boxes are not wrong.

My understanding is that large bounding boxes produce more workload on the servers. I assume that the rendering servers are clever enough so that they don't mark every tile as dirty, but I think I found a lot of noise in minutely diffs for e.g. Germany the last time I tried to use them (some years ago, might have changed since)
At least they are annoying when you look for changes in your area and find lots of huge bounding boxes without any change in your area.

If the users don't think it's necessary to work in a small area only then this is perfectly fine.

Correct for users which are aware that they don't work in a small area.
When looking at errors reported by QA tools you may forget to save a local change before looking at the next problem.
This is likely to produce unintended huge bounding boxes with a few changes.

comment:16 in reply to:  15 Changed 9 months ago by stoecker

Replying to GerdP:

Why? Large bounding boxes are not wrong.

My understanding is that large bounding boxes produce more workload on the servers. I assume that the rendering servers are clever enough so that they don't mark every tile as dirty, but I think I found a lot of noise in minutely diffs for e.g. Germany the last time I tried to use them (some years ago, might have changed since)

I doubt that the changeset bounding box has any influence here. If I remember right these mechanisms are solely based on the data itself.

comment:17 Changed 9 months ago by mkoniecz

I use JOSM outside the OSM world for a project as data display tool

I am not convinced that usability in this role should matter at all for an OSM editor. Yes, I also use JOSM as viewer of aerial imagery and for looking at GPX files - but complaining about interface for OSM editing appearing in an OSM editor seems a weird complaint. Or is JOSM supposed to be growing into QGIS and become a general purpose geodata tool?

Why? Large bounding boxes are not wrong.

I would say that not neded continent-size bounding boxes are wrong in the same way as missing changeset description or using "hsad8hy8dfyfw8" as a changeset description. And nearly all of them are not needed.

comment:18 in reply to:  17 Changed 9 months ago by stoecker

Replying to mkoniecz:

I use JOSM outside the OSM world for a project as data display tool

I am not convinced that usability in this role should matter at all for an OSM editor.

That was an example indicating the situation. The same amount of notes also disturbs with OSM usage.

Yes, I also use JOSM as viewer of aerial imagery and for looking at GPX files - but complaining about interface for OSM editing appearing in an OSM editor seems a weird complaint. Or is JOSM supposed to be growing into QGIS and become a general purpose geodata tool?

JOSM already is a GIS tool. We have a focus on OSM, but no limitation to OSM use. That's the philosophy for a long time now, but that's anyway off-topic.

Changed 9 months ago by simon04

Attachment: 2020-06-16-181434.png added

comment:19 Changed 9 months ago by simon04

What about a less intrusive way of showing the warning? We can also thank the mappers for providing a (hopefully good) changeset comment:


comment:20 in reply to:  19 Changed 9 months ago by stoecker

Replying to simon04:

What about a less intrusive way of showing the warning? We can also thank the mappers for providing a (hopefully good) changeset comment:

That's an idea, the text can change from please to thanks and orange to green when users follow the recommendation (so the UI does not flicker due to vanishing texts).

When we have some more variants for no comment, short comment, ... then your can even have a game play feature which may make fun instead of beeing annoying (probably with easter eggs - like a text "You are really nice" when someone enters "josm is wonderful" or similar :-)

"please consider splitting your changes" better would be "please consider uploading more often".

comment:21 Changed 9 months ago by simon04

Owner: changed from team to simon04
Status: reopenednew

:-)

I'll work on a patch …

comment:22 Changed 9 months ago by Klumbumbus

It would be great too if hovering or clicking the "The bounding box of this changeset is xxxkm²..." text would display a minimap showing the bbox as rectangle. Because it might be hard to imagine if e.g. 50km² is ok or not ok, i.e. is 50km² suspicious to be accidently include changes in another city or not?

Changed 9 months ago by simon04

Attachment: 2020-06-16-234254.png added

Changed 9 months ago by simon04

Attachment: 2020-06-16-234314.png added

comment:23 Changed 9 months ago by simon04

Progress has been made, but then I got stuck with #19399

comment:24 Changed 8 months ago by simon04

In 16672/josm:

see #19381 - Upload dialog: make warnings less intrusive

comment:25 Changed 8 months ago by simon04

Resolution: fixed
Status: newclosed

In 16673/josm:

fix #19381 - Upload dialog: warn about large bounding box

Changed 8 months ago by simon04

Attachment: 2020-06-17-213853.png added

comment:26 Changed 8 months ago by simon04


comment:27 Changed 8 months ago by stoecker

Colors should use preferences, so they can be changed.

comment:28 Changed 8 months ago by stoecker

Resolution: fixed
Status: closedreopened

comment:29 Changed 8 months ago by simon04

Resolution: fixed
Status: reopenedclosed

In 16675/josm:

fix #19381 - AbstractTextComponentValidator: use NamedColorProperty


The world upside down – red/pink is good, green is bad: ;-)

Last edited 8 months ago by simon04 (previous) (diff)

Changed 8 months ago by simon04

Attachment: 2020-06-17-230219.png added

comment:30 Changed 8 months ago by skyper

Mmh, I am not sure if some modified relations like superroute should be excluded in general.

Is it possible to remove some objects from the list? You can select them and zoom plus highlighting works but you can not remove them from the list and have the action automatically changed to "upload selection". This would be nice. New ticket?

comment:31 Changed 8 months ago by stoecker

Resolution: fixed
Status: closedreopened

Need to reopen again.

The area you calculate is square degree. That's no usable unit. Geometry has a function getAreaAndPerimeter() for 4 nodes to get the area in square meters (see multipolygonArea() for projection).

The value to check against should be something like 900 square km (that would be a 30x30km area). That's already small for my typical daily tours. Probably 2500 would be better (50x50).

comment:32 Changed 8 months ago by Klumbumbus

The dialog often flickers for one frame when typing. See attachement.

Changed 8 months ago by Klumbumbus

Attachment: 19381flicker.gif added

comment:33 Changed 8 months ago by Klumbumbus

Before we had, e.g.

message.upload_comment_is_empty_or_very_short=false
message.upload_comment_is_empty_or_very_short.value=3
message.upload_source_is_empty=false
message.upload_source_is_empty.value=3

("Do not show again" dialog)

This doesn't work anymore. Is there a replacement? As a mapper who is aware of the shown texts I would like to hide them because they use a lot of space.

comment:34 Changed 8 months ago by simon04

Milestone: 20.06

comment:35 in reply to:  31 Changed 8 months ago by simon04

Replying to stoecker:

The area you calculate is square degree. That's no usable unit. Geometry has a function getAreaAndPerimeter() for 4 nodes to get the area in square meters (see multipolygonArea() for projection).

I was aware of this fact. I took the calculation from org.openstreetmap.josm.gui.download.OSMDownloadSource.OsmDataDownloadType#isDownloadAreaTooLarge

comment:36 Changed 8 months ago by simon04

In 16695/josm:

see #19381 - Upload dialog: allow to disable text validation

  • message.upload_comment_is_empty_or_very_short
  • message.upload_source_is_empty

comment:37 Changed 8 months ago by simon04

In 16696/josm:

see #19381 - Upload dialog: (re)validate when showing the dialog

comment:38 Changed 8 months ago by simon04

In 16697/josm:

see #19381 - Upload dialog: avoid unnecessary UI updates when validating

comment:39 Changed 8 months ago by Klumbumbus

Thanks. comment:32 and 33 work fine for me now, but I noticed another problem. A lot of the text is missing if you add or remove comments and then close and reopen the dialog. See following screencast:

Changed 8 months ago by Klumbumbus

Attachment: 19381cutoff.gif added

comment:40 Changed 8 months ago by stoecker

Milestone: 20.0620.07

comment:41 Changed 8 months ago by Klumbumbus

Not a blocker but it would be good if the cutoff could be fixed before the 20.06 release.

comment:42 Changed 8 months ago by stoecker

Simon?

comment:43 Changed 8 months ago by simon04

In 16730/josm:

see #19381 - Upload dialog: hopefully fix layout issue

comment:44 Changed 8 months ago by Klumbumbus

That works fine for me (except for the case when you switch tabs in the upload dialog too, which is a minor problem).

comment:45 Changed 8 months ago by simon04

Owner: changed from simon04 to team
Status: reopenednew

comment:46 Changed 7 months ago by simon04

Milestone: 20.0720.06
Resolution: fixed
Status: newclosed

The main feature has landed in 20.06. Let's open separate ticket for the outstanding issues (if any).

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.