[PATCH] Way "modified" flag should be based on node IDs, not coordinates

[mattmccutchen]

Generally, if the user changes a primitive and then manually reverts the change (without using "undo"), JOSM doesn't automatically clear the modified flag, but any command that re-downloads the primitive (such as "Update modified") will clear the modified flag (see #3384 and ticket:17983#comment:1). However, for a way, the decision to clear the modified flag is based on the coordinates of the nodes in the way. This does not match the OSM data model, which stores only the node IDs. As a result, if a node has been moved, the way can get stuck in the "modified" state. This is confusing when I'm reviewing my own change by searching for modified objects, and a new version can even get uploaded that has no diff from the previous version. The latter happened to me in ​changeset 184037358 (​affected way 1, ​affected way 2).

There's also a reverse scenario in which there are two nodes N1 and N2 at the same coordinates and a way W that contained N1 is modified to contain N2 instead. An "Update modified" will mark W as unmodified because the node coordinates match. At that point, the history dialog will show no diff and an upload will not upload W because it is marked unmodified. But internally, W still contains N2! If the user then makes some other change to W and uploads it, the replacement of N1 with N2 will get uploaded along with the other change. This is potentially dangerous because a change is uploaded potentially without the user's knowledge. Fortunately, I think the scenario is rare because duplicate nodes are discouraged. Also fortunately, if W is updated a second time ("Update modified" would not do it, but "Update data" would, for example), it gets reverted to contain N1. I'm going to focus on the first scenario in this ticket.

What steps will reproduce the problem?

1. Download some data.
2. Move a node that belongs to a way.
3. Search for modified. Only the node is selected.
4. Change a tag of the way, then change it back manually (not using "undo").
5. Search for modified. Both the node and the way are selected.
6. Perform the "Update modified" command.
7. Search for modified.

What is the expected result?

In step 7, only the node is selected.

What happens instead?

Both the node and the way are selected.

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

The behavior is coming from ​this code in Way.hasEqualSemanticAttributes. Changing Way.hasEqualSemanticAttributes to compare node IDs instead of coordinates (as ​it does for relations) fixes the bug. But I wonder why Way.hasEqualSemanticAttributes was written to compare coordinates. Are there other use cases that rely on it? A wild guess: do some users rely on that behavior to deduplicate "new" ways when merging layers? If so, a possible compromise might be to have Way.hasEqualSemanticAttributes compare the coordinates of a node if it is "new" and the ID if it is not "new".

To help get things moving, I'll attach a patch that just changes Way.hasEqualSemanticAttributes to compare node IDs. However, even if that approach is acceptable, I think it might help to have at least one unit test of the scenario from this ticket to help ensure that someone doesn't change the code back without being aware of the scenario. Any advice on how to write such a test would be appreciated.

Side remark: I noticed that the map paint style for "Modified objects" shows the way as modified starting from step 2. I haven't figured out why that's happening, but I think it's a separate issue and I'll file a separate ticket if I care about it.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2026-10-12 17:25:48 +0200 (Mon, 12 Oct 2026)
Revision:19647
Build-Date:2026-10-13 01:30:50
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (19647 en) Linux Fedora Linux 44 (Forty Four)
Memory Usage: 240 MB / 4072 MB (38 MB allocated, but free)
Java version: 25.0.4.1+1, Red Hat, Inc., OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel
Screen: :0.0 1920x1080x[Multi depth]@46Hz (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→16×16, 32×32→32×32
Environment variable LANG: en_US.utf8
System property file.encoding: UTF-8
System property sun.jnu.encoding: UTF-8
Locale info: en_US
Numbers with default locale: 1234567890 -> 1234567890
Desktop environment: X-QUBES
VM arguments: [--add-exports=java.base/sun.security.action=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.plugins.jpeg=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED, -Djosm.home=<josm.pref>]
Dataset consistency test: No problems found

[GerdP]

Didn't test it yet. I don't like the idea to change the behaviour of such an old public method. Is there really no unit test which fails with this change? If I got that right the code was introduced for merging layers, which really is a can of worms.

[mattmccutchen]

I'm coming back to this after being busy for a while. Indeed, I neglected to run the unit tests on my patch, and DataSetMergerTest.testWaySimpleTwoWaysWithNoIdNodesWithoutId fails because it's expecting "new" nodes to be compared by semantic attributes. However, I'm still not clear on why the behavior this test is checking for is desirable. What's the end-to-end user scenario or workflow that requires this behavior? If we keep the behavior, it would be good to document why.

In any case, since it sounds like it's going to be more realistic to keep the old behavior in the case of new nodes than to fully assess the potential for breakage from changing it, I went ahead and updated my patch to implement the compromise I mentioned, where Way.hasEqualSemanticAttributes compares the IDs of non-"new" nodes and the semantic attributes of "new" nodes. I also added tests demonstrating the bug being fixed. Assuming there's agreement about the approach, the only obvious remaining issue with the patch is the TODO to document the reason for the behavior for "new" nodes.

[mattmccutchen]

Replying to mattmccutchen:

Side remark: I noticed that the map paint style for "Modified objects" shows the way as modified starting from step 2. I haven't figured out why that's happening, but I think it's a separate issue and I'll file a separate ticket if I care about it.

For the record: This happens because the "Modified objects" style uses the node:modified < way::modified_layer selector to highlight any way that contains a modified node. I presume that behavior of the paint style is intentional, but it's not what I want personally, so I made my own version of the paint style (which I may eventually share) without this selector.

[stoecker]

Milestone renamed