Changeset 18783 in josm

Timestamp:

2023-07-26T22:06:14+02:00 (2 years ago)

Author:

taylor.smock

Message:

Fix #23081: NoSuchElementException in ConflictDialog.ResolveToAction#actionPerformed

This occurs when one of the Resolve to (my|their) versions actions is called
when no conflict is selected.

There are two distinct bugs:

• isConflictSelected did not check to make certain that the selection index made sense
• ResolveActions were not enabled/disabled when the popup menu became visible, depending upon whether the current selection was valid

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/dialogs/ConflictDialog.java (modified) (3 diffs)
• test/unit/org/openstreetmap/josm/gui/dialogs/ConflictDialogTest.java (modified) (3 diffs)

trunk/src/org/openstreetmap/josm/gui/dialogs/ConflictDialog.java

@@ -322 +322 @@
     private synchronized boolean isConflictSelected() {
         final ListSelectionModel selModel = lstConflicts.getSelectionModel();
-        return selModel.getMinSelectionIndex() >= 0 && selModel.getMaxSelectionIndex() >= selModel.getMinSelectionIndex();
+        final int minSelectionIndex = selModel.getMinSelectionIndex();
+        final int maxSelectionIndex = selModel.getMaxSelectionIndex();
+        final int maxIndex = conflicts.size();
+        // if minSelectionIndex < 0, nothing is selected
+        // if minSelectionIndex > maxIndex, then nothing is selected (we are operating with an old selection context, most likely)
+        // if maxSelectionIndex < minSelectionIndex, _something_ funny is going on. Or there was a typo in the original code.
+        return minSelectionIndex >= 0 && maxIndex > minSelectionIndex && maxSelectionIndex >= minSelectionIndex;
     }
 
@@ -369 +375 @@
             btnResolveMy.setVisible(ExpertToggleAction.isExpert());
             btnResolveTheir.setVisible(ExpertToggleAction.isExpert());
+            ((ResolveAction) btnResolveMy.getAction()).valueChanged(null);
+            ((ResolveAction) btnResolveTheir.getAction()).valueChanged(null);
         }
 
@@ -529 +537 @@
                 }
             }
-            UndoRedoHandler.getInstance().add(new SequenceCommand(name, commands));
+                UndoRedoHandler.getInstance().add(new SequenceCommand(name, commands));
             refreshView();
         }

trunk/test/unit/org/openstreetmap/josm/gui/dialogs/ConflictDialogTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.gui.dialogs;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import java.awt.Color;
 import java.awt.image.BufferedImage;
 
-import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.api.Test;
 import org.openstreetmap.josm.data.coor.LatLon;
@@ -19 +18 @@
 import org.openstreetmap.josm.gui.dialogs.ConflictDialog.ConflictPainter;
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
-import org.openstreetmap.josm.testutils.JOSMTestRules;
-
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import org.openstreetmap.josm.testutils.annotations.Main;
+import org.openstreetmap.josm.testutils.annotations.Projection;
 
 /**
  * Unit tests of {@link ConflictDialog} class.
  */
+@Main
+@Projection
 class ConflictDialogTest {
-
-    /**
-     * Setup tests
-     */
-    @RegisterExtension
-    @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    public JOSMTestRules test = new JOSMTestRules().main().projection();
-
     /**
      * Unit test of {@link ConflictDialog#ConflictDialog}.
@@ -40 +32 @@
     @Test
     void testConflictDialog() {
-        assertNotNull(new ConflictDialog());
+        assertDoesNotThrow(ConflictDialog::new);
     }