Changeset 18842 in josm for trunk/src/org/openstreetmap

Timestamp:

2023-09-25T17:34:00+02:00 (16 months ago)

Author:

taylor.smock

Message:

Fix #23191: NPE in AddTagsDialog

This is caused when a new layer is added via Remote Control while the add tag
dialog is open.

File:

• trunk/src/org/openstreetmap/josm/gui/dialogs/properties/TagEditHelper.java (modified) (12 diffs)

trunk/src/org/openstreetmap/josm/gui/dialogs/properties/TagEditHelper.java

@@ -73 +73 @@
 import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
 import org.openstreetmap.josm.data.osm.Tag;
+import org.openstreetmap.josm.data.osm.Tagged;
 import org.openstreetmap.josm.data.osm.search.SearchCompiler;
 import org.openstreetmap.josm.data.osm.search.SearchParseError;
@@ -82 +83 @@
 import org.openstreetmap.josm.data.preferences.StringProperty;
 import org.openstreetmap.josm.data.tagging.ac.AutoCompletionItem;
+import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
 import org.openstreetmap.josm.gui.ExtendedDialog;
 import org.openstreetmap.josm.gui.IExtendedDialog;
@@ -278 +280 @@
         try {
             activeDataSet.beginUpdate();
-            sel = OsmDataManager.getInstance().getInProgressSelection();
-            if (Utils.isEmpty(sel))
+            Collection<OsmPrimitive> selection = OsmDataManager.getInstance().getInProgressSelection();
+            this.sel = selection;
+            if (Utils.isEmpty(selection))
                 return;
 
@@ -287 +290 @@
 
             addDialog.destroyActions();
-            if (addDialog.getValue() == 1)
-                addDialog.performTagAdding();
-            else
+            // Remote control can cause the selection to change, see #23191.
+            if (addDialog.getValue() == 1 && (selection == sel || warnSelectionChanged())) {
+                addDialog.performTagAdding(selection);
+            } else {
                 addDialog.undoAllTagsAdding();
+            }
         } finally {
             activeDataSet.endUpdate();
@@ -373 +378 @@
     public void loadTagsIfNeeded() {
         loadTagsToIgnore();
-        if (PROPERTY_REMEMBER_TAGS.get() && recentTags.isEmpty()) {
+        if (Boolean.TRUE.equals(PROPERTY_REMEMBER_TAGS.get()) && recentTags.isEmpty()) {
             recentTags.loadFromPreference(PROPERTY_RECENT_TAGS);
         }
@@ -407 +412 @@
      */
     public void saveTagsIfNeeded() {
-        if (PROPERTY_REMEMBER_TAGS.get() && !recentTags.isEmpty()) {
+        if (Boolean.TRUE.equals(PROPERTY_REMEMBER_TAGS.get()) && !recentTags.isEmpty()) {
             recentTags.saveToPreference(PROPERTY_RECENT_TAGS);
         }
@@ -449 +454 @@
             return selectedItem.toString();
         return getEditItem(cb);
+    }
+
+    /**
+     * Warn user about a selection change
+     * @return {@code true} if the user wants to apply the tag change to the old selection
+     */
+    private static boolean warnSelectionChanged() {
+        return ConditionalOptionPaneUtil.showConfirmationDialog("properties.selection-changed",
+                MainApplication.getMainFrame(),
+                tr("Data selection has changed since the dialog was opened"),
+                tr("Apply tag change to old selection?"),
+                JOptionPane.YES_NO_OPTION, JOptionPane.WARNING_MESSAGE, JOptionPane.YES_OPTION);
     }
 
@@ -506 +523 @@
                 /**
                  * This hack allows the comboboxes to have their own orientation.
-                 *
+                 * <p>
                  * The problem is that
                  * {@link org.openstreetmap.josm.gui.ExtendedDialog#showDialog ExtendedDialog} calls
                  * {@code applyComponentOrientation} very late in the dialog construction process
                  * thus overwriting the orientation the components have chosen for themselves.
-                 *
+                 * <p>
                  * This stops the propagation of {@code applyComponentOrientation}, thus all
                  * components may (and have to) set their own orientation.
@@ -794 +811 @@
                 /**
                  * This hack allows the comboboxes to have their own orientation.
-                 *
+                 * <p>
                  * The problem is that
                  * {@link org.openstreetmap.josm.gui.ExtendedDialog#showDialog ExtendedDialog} calls
                  * {@code applyComponentOrientation} very late in the dialog construction process
                  * thus overwriting the orientation the components have chosen for themselves.
-                 *
+                 * <p>
                  * This stops the propagation of {@code applyComponentOrientation}, thus all
                  * components may (and have to) set their own orientation.
@@ -1187 +1204 @@
          */
         public final void performTagAdding() {
+            Collection<OsmPrimitive> selection = sel;
+            if (!Utils.isEmpty(selection)) {
+                performTagAdding(selection);
+            }
+        }
+
+        /**
+         * Read tags from comboboxes and add it to all selected objects
+         * @param selection The selection to perform tag adding on
+         * @since 18842
+         */
+        private void performTagAdding(Collection<OsmPrimitive> selection) {
             String key = getEditItem(keys);
             String value = getEditItem(values);
             if (key.isEmpty() || value.isEmpty())
                 return;
-            for (OsmPrimitive osm : sel) {
+            for (Tagged osm : selection) {
                 String val = osm.get(key);
                 if (val != null && !val.equals(value)) {
@@ -1203 +1232 @@
             }
             recentTags.add(new Tag(key, value));
-            valueCount.put(key, new TreeMap<String, Integer>());
+            valueCount.put(key, new TreeMap<>());
             AutoCompletionManager.rememberUserInput(key, value, false);
             commandCount++;
-            UndoRedoHandler.getInstance().add(new ChangePropertyCommand(sel, key, value));
+            UndoRedoHandler.getInstance().add(new ChangePropertyCommand(selection, key, value));
             changedKey = key;
             clearEntries();
@@ -1216 +1245 @@
         }
 
+        /**
+         * Undo all tag add commands that this dialog has created
+         */
         public void undoAllTagsAdding() {
             UndoRedoHandler.getInstance().undo(commandCount);