Modify

Opened 4 weeks ago

Last modified 8 days ago

#19381 reopened enhancement

Warn users about making changesets with unreasonably large bounding boxes

Reported by: mkoniecz Owned by: simon04
Priority: normal Milestone: 20.07
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 3 weeks ago.
2020-06-16-234254.png (19.8 KB) - added by simon04 3 weeks ago.
2020-06-16-234314.png (17.6 KB) - added by simon04 3 weeks ago.
2020-06-17-213853.png (16.3 KB) - added by simon04 3 weeks ago.
2020-06-17-230219.png (16.3 KB) - added by simon04 3 weeks ago.
19381flicker.gif (160.3 KB) - added by Klumbumbus 3 weeks ago.
19381cutoff.gif (963.3 KB) - added by Klumbumbus 2 weeks ago.

Download all attachments as: .zip

Change History (51)

comment:1 Changed 4 weeks ago by mkoniecz

Description: modified (diff)

comment:2 Changed 4 weeks 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 4 weeks 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 4 weeks 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 4 weeks ago by mkoniecz (previous) (diff)

comment:5 Changed 4 weeks 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 4 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks 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 3 weeks ago by simon04

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

comment:19 Changed 3 weeks 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 3 weeks 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 3 weeks ago by simon04

Owner: changed from team to simon04
Status: reopenednew

:-)

I'll work on a patch …

comment:22 Changed 3 weeks 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 3 weeks ago by simon04

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

Changed 3 weeks ago by simon04

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

comment:23 Changed 3 weeks ago by simon04

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

comment:24 Changed 3 weeks ago by simon04

In 16672/josm:

see #19381 - Upload dialog: make warnings less intrusive

comment:25 Changed 3 weeks ago by simon04

Resolution: fixed
Status: newclosed

In 16673/josm:

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

Changed 3 weeks ago by simon04

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

comment:26 Changed 3 weeks ago by simon04


comment:27 Changed 3 weeks ago by stoecker

Colors should use preferences, so they can be changed.

comment:28 Changed 3 weeks ago by stoecker

Resolution: fixed
Status: closedreopened

comment:29 Changed 3 weeks 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 3 weeks ago by simon04 (previous) (diff)

Changed 3 weeks ago by simon04

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

comment:30 Changed 3 weeks 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 3 weeks 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 3 weeks ago by Klumbumbus

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

Changed 3 weeks ago by Klumbumbus

Attachment: 19381flicker.gif added

comment:33 Changed 3 weeks 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 3 weeks ago by simon04

Milestone: 20.06

comment:35 in reply to:  31 Changed 3 weeks 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 3 weeks 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 3 weeks ago by simon04

In 16696/josm:

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

comment:38 Changed 3 weeks ago by simon04

In 16697/josm:

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

comment:39 Changed 2 weeks 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 2 weeks ago by Klumbumbus

Attachment: 19381cutoff.gif added

comment:40 Changed 9 days ago by stoecker

Milestone: 20.0620.07

comment:41 Changed 9 days 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 days ago by stoecker

Simon?

comment:43 Changed 8 days ago by simon04

In 16730/josm:

see #19381 - Upload dialog: hopefully fix layout issue

comment:44 Changed 8 days 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).

Modify Ticket

Change Properties
Set your email in Preferences
Action
as reopened The owner will remain simon04.
as The resolution will be set.
to The owner will be changed from simon04 to the specified user.
The owner will change to mkoniecz
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket

Add Comment


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

 
Note: See TracTickets for help on using tickets.