[patch] After downloading more notes, clicking on first note points to previous first one

[angoca]

What steps will reproduce the problem?

1. Select a note in OSM, and click edit on JOSM (to download the map data).
2. Change to JOSM, and download the notes in that area.
3. Resolve the selected note.
4. Select another note in OSM, and click edit on JOSM.
5. Click on the first note of the Notes view, it will point to the previous note. Not the currently selected.

What is the expected result?

JOSM should do zoom to the current selected note, not the previous one.

What happens instead?

JOSM zooms to the previous one.

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

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-11-01 23:05:46 +0100 (Mon, 01 Nov 2021)
Build-Date:2021-11-01 22:25:18
Revision:18303
Relative:URL: ^/trunk

Identification: JOSM/1.5 (18303 en) Mac OS X 10.15.7
OS Build number: Mac OS X 10.15.7 (19H1519)
Memory Usage: 2019 MB / 3641 MB (250 MB allocated, but free)
Java version: 1.8.0_311-b11, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.apple.laf.AquaLookAndFeel
Screen: Display 69731270 1440×900 (scaling 1.00×1.00)
Maximum Screen Size: 1440×900
Best cursor sizes: 16×16→16×16, 32×32→32×32
System property file.encoding: UTF-8
System property sun.jnu.encoding: UTF-8
Locale info: en_CO
Numbers with default locale: 1234567890 -> 1234567890
VM arguments: [-Djava.security.policy=file:<java.home>/lib/security/javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>/bin, -Djava.security.manager, -Djnlpx.origFilenameArg=${HOME}/Library/Application Support/Oracle/Java/Deployment/cache/6.0/56/1ee8cfb8-2db50015, -Djnlpx.remove=false, -Dsun.awt.warmup=true, -Djava.util.Arrays.useLegacyMergeSort=true, -Dmacosx.jnlpx.dock.name=JOSM, -Dmacosx.jnlpx.dock.icon=${HOME}/Library/Application Support/Oracle/Java/Deployment/cache/6.0/25/4c122699-7a1c6b48.icns, -Djnlp.application.href=https://josm.openstreetmap.de/download/josm.jnlp , -Djnlpx.jvm="<java.home>/bin/java"]
Dataset consistency test: No problems found

Plugins:
+ DirectDownload (35640)
+ DxfImport (1014)
+ FastDraw (35864)
+ ImportImagePlugin (35567)
+ ImproveOsm (215)
+ Mapillary (2.0.0-alpha.43)
+ PicLayer (1.0.1)
+ apache-commons (35524)
+ apache-http (35589)
+ buildings_tools (35823)
+ continuosDownload (99)
+ ejml (35458)
+ fieldpapers (v0.5.0)
+ geotools (35458)
+ imagery_offset_db (35640)
+ jaxb (35543)
+ jna (35662)
+ jts (35458)
+ log4j (35852)
+ opendata (35803)
+ pdfimport (35640)
+ photo_geotagging (35783)
+ undelete (35848)
+ utilsplugin2 (35856)

Validator rules:
+ ${HOME}/github/ColombianJosmValidator/Rules_Colombian-specific.validator.mapcss

Last errors/warnings:
- 138600.470 W: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: java.net.SocketException: Socket closed
- 138600.472 E: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: java.net.SocketException: Socket closed
- 138601.104 W: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: java.net.SocketException: Socket closed
- 138601.105 E: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: java.net.SocketException: Socket closed
- 143842.226 W: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: java.net.SocketException: Socket closed
- 143842.226 E: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: java.net.SocketException: Socket closed
- 168272.347 W: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: java.net.SocketException: Socket closed
- 168272.348 E: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: java.net.SocketException: Socket closed
- 168272.940 W: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: java.net.SocketException: Socket closed
- 168272.944 E: javax.net.ssl.SSLException: java.net.SocketException: Socket closed. Cause: java.net.SocketException: Socket closed

[angoca]

As you can see in the attached image, the selected one is 830631 from the Notes panel, but the one in the map is 828372. This was before downloading more notes.
The way to download more notes is the button on the Notes panel.

[gaben]

Can be a note caching/networking issue, but the steps are not clear for me, cannot reproduce. Please clarify.

[angoca]

I created a video in YouTube that better explain the problem: ​https://youtu.be/YUE_F29aXrU

[gaben]

I see, thank you! I'm looking into it.

[gaben]

Ah found the cause. It's only a GUI thing, fortunately. At ​https://youtu.be/YUE_F29aXrU?t=65 you have the selection on ​note/1003367. After download, the selection goes onto ​note/1007708, but the selection dataset not updated internally.

The following patch fixes the issue.

src/org/openstreetmap/josm/data/notes/Note.java

@@ -29 +29 @@
 
     /**
      * Sorts notes in the following order:
-     * 1) Open notes
-     * 2) Closed notes
-     * 3) New notes
+     * <ol>
+     *     <li>Open notes</li>
+     *     <li>Closed notes</li>
+     *     <li>New notes</li>
+     * </ol>
      * Within each subgroup it sorts by ID
      */
     public static final Comparator<Note> DEFAULT_COMPARATOR = (n1, n2) -> {

src/org/openstreetmap/josm/gui/dialogs/NotesDialog.java

@@ -205 +205 @@
     public void setNotes(Collection<Note> noteList) {
         model.setData(noteList);
         updateButtonStates();
+        selectionChanged();
         this.repaint();
     }

[gaben]

The side effect is that the selection "locks" the list view to the selected note. So downloading new notes no longer moves the selection to the top of the list in the Notes dialog.

It applies only if there was a selected note in the list.

[taylor.smock]

In 18479/josm:

Fix #21600: When setting/updating notes, fire selectionChanged events (patch by gaben)

[taylor.smock]

In 18483/josm:

NotesDialog: Fix test broken in r18479, see #21600

The test ensures that updates to the dialog do not
cause a IOOBE, which often only occurs when a note
is closed and the user is filtering for open notes.

[gaben]

Oopsie, thank you! I haven't ran tests at all.

[taylor.smock]

No problem -- I should have run the tests as well, especially considering the age of the ticket. I just applied it locally, and checked to make certain that things weren't broken in standard use, and validated that the fix appeared to work.

Anyway, this is one of the reasons why I like unit tests, although those are kinda hard for UI issues.

The failing test was fairly recent, so given the age of this ticket, I wouldn't be surprised that it failed -- see #21558 (r18454).