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

[aceman]

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

[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.

[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.

[GerdP]

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.

[skyper]

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

[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.

[GerdP]

remove support for downloading changeset URLs with OpenLocationAction

[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.

[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.

[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!".

[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.

[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.

[GerdP]

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

[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.

[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.

[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.

[GerdP]

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

[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".

[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.

[taylor.smock]

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