Modify

Opened 4 months ago

Last modified 4 months ago

#21904 new enhancement

[Patch] Pasting a changeset URL causes the changeset's objects and their historys to be downloaded

Reported by: aceman Owned by: team
Priority: normal Milestone:
Component: Core Version: tested
Keywords: changeset url download layer Cc:

Description

Hi, I have noticed a very dangerous scenario while editing in JOSM:

  1. Have some data for editing in a layer in JOSM
  2. Get some changeset URL, e.g. by showing history of an object (Ctrl-H) and right click on the changeset of the last change on the object
  3. Now you have the changeset URL in the clipboard and can paste to your browser or so.
  4. However, if you by mistake paste this into JOSM (Ctrl+V or Ctrl+Shift+V) [e.g. thinking you have some other tags/object in the clipboard from previous editing], JOSM does start to load the changeset and seems to do some operation on it. I is not just loading the data from it (maybe it is), as trying to upload the layer now causes objects from the changeset to be uploaded with changes and gets conflicts from the server, which has newer versions of the objects.

Whether the changeset is reverted to before the changeset, or just the objects are restored to the versions from the changeset I don't know. But this must not happen as it mixes the changeset into your current work and then it is impossible to restore the layer to only your changes and not these unintended changes. The whole layer has to be cancelled and work is lost.

Can anything be done with this? If this is intentional behaviour can at least a dialog warn about it?
Thanks

Attachments (2)

21904.patch (1.2 KB) - added by GerdP 4 months ago.
remove support for downloading changeset URLs with OpenLocationAction
21904.2.patch (5.2 KB) - added by taylor.smock 3 weeks ago.
Add default method providesOldData, return true in DownloadOsmChangeTask, and use the return to determine if a confirmation window should be shown to the user

Download all attachments as: .zip

Change History (21)

comment:1 Changed 4 months ago by GerdP

OK, nothing is reverted (at least the reverter plugin is not involved). The pasted url is downloaded and then the history for all objects in the changeset data is downloaded. No idea in what situation this would be useful. I agree that this data should not be added to an existing layer which already contains data without any confirmation from the user.

comment:2 Changed 4 months ago by aceman

Ok, if it is not reverted, but the objects seem to be then uploaded in the version as in that changeset. As I get conflicts against the data on server and the conflict resolving dialog shows I have older versions of the objects than on the server. So it is not just the data from changeset is downloaded into the current layer. Old versions of the objects are downloaded.

comment:3 Changed 4 months ago by GerdP

Priority: majornormal
Type: defectenhancement

Well, the data in a changeset can be similar to the current data in OSM or much older, this depends on the age of the changeset. If the changeset contains version 4 of a way and current version is 6 or higher you should see conflicts. I don't know why there is support for pasting urls into a data layer, esp. reg. urls of changesets, but this is not a new feature. I've just tried with version r11747 and see the same behaviour.

comment:4 Changed 4 months ago by skyper

Downloading the history of all objects without user interaction is not nice, see #21563.

comment:5 Changed 4 months ago by skyper

Keywords: changeset url download layer added
Summary: Pasting a changeset URL causes the changeset to be revertedPasting a changeset URL causes the changeset's objects and their historys to be downloaded

comment:6 Changed 4 months ago by GerdP

I think this was introduced with r10604.
The paste action calls OpenLocationAction (Ctrl+L). My simple approach would be to remove the support for downloading changeset URLs from the list of supported sources in this action as I see no use case for this.
Question is if other URLs should be pasted silently. I didn't even know that this works before reading this ticket.

Changed 4 months ago by GerdP

Attachment: 21904.patch added

remove support for downloading changeset URLs with OpenLocationAction

comment:7 Changed 4 months ago by stoecker

Hmm. There are many other ways to get older data into JOSM. I don't see why we should handle this case special. Copy&Paste of old data will have the same result in all cases.

Last edited 4 months ago by stoecker (previous) (diff)

comment:8 Changed 4 months ago by GerdP

What would you suggest to do? I think the special case is that the right click on the changeset number fills the copy&paste buffer with the URL. This is a rather special behaviour.

comment:9 Changed 4 months ago by stoecker

Hmm. Right. It's probably more dangerous than the other OpenLocation tasks. Can we add a warning to the DownloadOsmChangesetTask without making other parts of JOSM awkward? Something like "Be careful with the loaded data. It's not the latest data!".

comment:10 Changed 4 months ago by skyper

I did not know this feature and it is poorly documented. I am lucky to have "download to new layer" always set and to not change it as I still miss the old general setting to always download into new layer.

I wonder if it would be better to always display the OpenLocation dialog instead of directly downloading urls. We could have an advanced preferences to keep the old behavior but disabled by default.
Talking about changesets, I would always show the OpenLocation dialog including warnings. Downloading the changeset needs quite some resources and it current form is a mix of old and current objects' versions as i.e. way nodes not included in the CS are not downloaded with their old version.
Personally, I use reverter plugin and undo to download a changeset. Then I have all objects with latest version.

By the way, as paste does not work without a layer, at least one data layer is needed.

comment:11 in reply to:  9 Changed 4 months ago by aceman

Replying to stoecker:

Hmm. Right. It's probably more dangerous than the other OpenLocation tasks. Can we add a warning to the DownloadOsmChangesetTask without making other parts of JOSM awkward? Something like "Be careful with the loaded data. It's not the latest data!".

Yes, but the user must be aware this is happening and there is some "not the latest data". From observing the loading dialogs which load the changeset URL, this is not obvious. Also, user may not want to mix his current edits with this outdated changeset data.

Maybe it is useful to load an changeset URL into an empty layer (but then user also must be made aware by JOSM, than old version of objects will be upload). But it is strange to mix old data into new data.

I may have unknowingly reverted some data to old versions (which didn't have conflicts), due to this very dangerous behaviour.

comment:12 Changed 4 months ago by GerdP

Should I commit my patch or does somebody know a better solution?

comment:13 Changed 4 months ago by skyper

The problem of possibly old data is the same if you use the download location dialog.
I'd prefer to always display this download if pasting an URL containing /changeset/.
Additionally, a warning about the possibly old data is useful in all cases.

comment:14 Changed 4 months ago by GerdP

I don't understand. My patch disables the download of URLs containing changeset as well. The dialog for an invalid URl is shown when you try it.

comment:15 in reply to:  14 Changed 4 months ago by skyper

Replying to GerdP:

I don't understand. My patch disables the download of URLs containing changeset as well. The dialog for an invalid URl is shown when you try it.

Yes, it is a quick and easy fix, though, it disables a feature which did no harm under certain circumstances, see my comment 10

I rather would suggest to

  1. have a general advanced preferences (default: disabled) to enable the shortcut for Open Location when pasting an URL in mapview
  2. Some kind of info if old versions of objects are downloaded

Anyway, go a head and I will create a new ticket for my suggestions.

comment:16 Changed 4 months ago by skyper

Summary: Pasting a changeset URL causes the changeset's objects and their historys to be downloaded[Patch] Pasting a changeset URL causes the changeset's objects and their historys to be downloaded

comment:17 Changed 4 months ago by GerdP

I don't understand the comment:10. In what situation is it useful to be able to download a changeset content?

comment:18 Changed 4 months ago by skyper

If I want to take look at the objects at the moment of a changeset. It is sometimes useful to understand changes and for reverting. Unlike the action in changeset manager "Download object in its current state" it reverts all changes on the objects and does not create conflicts like reverter plugin does.

As written, I did not know the feature of the shortcut to Open Location neither what it does when "opening" a CS. Mean-while it was quite useful in two occasions where I had to look at changes made some month or in one case two years ago. Both time with only some small relations with low version number involved.

Playing around, in my eyes, the most dangerous part is the wrong object's version number. Unlike e.g. https://www.openstreetmap.org/api/0.6/node/663682975/1 the version number is a new version but it should be the old one as present in the changeset. Using the objects' version numbers of the changeset probably prevents a lot of object history requests.
Second, only the relations but not their members should be downloaded. Similar to #21910 current behavior is not that useful, especially with the "new" action "Download with Members" and the changes to "Download members".

comment:19 Changed 4 months ago by Woazboat

I have used this feature a few times in the past, so please don't remove it. A warning dialog/required user confirmation would be nice though when this results in mixing data into an existing layer.

Changed 3 weeks ago by taylor.smock

Attachment: 21904.2.patch added

Add default method providesOldData, return true in DownloadOsmChangeTask, and use the return to determine if a confirmation window should be shown to the user

Modify Ticket

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

Add Comment


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

 
Note: See TracTickets for help on using tickets.