Ticket #21408: 21408-4.patch

src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetItemGuiSupport.java

@@ -84 +84 @@
     }
 
     /**
-     * Returns true if all selected primitives matched this preset (before opening the dialog)
+     * Returns true if all selected primitives matched this preset (before opening the dialog).
+     * <p>
+     * This usually means that the preset dialog was opened from the Tags / Memberships panel as
+     * opposed to being opened by selection from the menu or toolbar or the search.
      *
      * @return true if the preset initially matched
      */

src/org/openstreetmap/josm/gui/tagging/presets/items/Combo.java

@@ -153 +153 @@
             p.add(combobox, GBC.eol().fill(GBC.HORIZONTAL)); // NOSONAR
         }
 
-        String valueToSelect = getInitialValue(usage);
-        if (valueToSelect != null) {
-            PresetListEntry selItem = find(valueToSelect);
-            if (selItem != null) {
-                combobox.setSelectedItem(selItem);
-            } else {
-                combobox.setText(valueToSelect);
-            }
+        String initialValue = getInitialValue(usage, support);
+        PresetListEntry selItem = find(initialValue);
+        if (selItem != null) {
+            combobox.setSelectedItem(selItem);
+        } else {
+            combobox.setText(initialValue);
         }
+
         combobox.addActionListener(l -> support.fireItemValueModified(this, key, getSelectedItem().value));
         combobox.addComponentListener(new ComponentListener());
 

src/org/openstreetmap/josm/gui/tagging/presets/items/ComboMultiSelect.java

@@ -22 +22 @@
 import javax.swing.ListCellRenderer;
 
 import org.openstreetmap.josm.data.osm.Tag;
+import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetItemGuiSupport;
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetSelector;
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresets;
 import org.openstreetmap.josm.gui.widgets.JosmListCellRenderer;
@@ -93 +94 @@
     /** Helps avoid duplicate list entries */
     protected final Map<String, PresetListEntry> seenValues = new TreeMap<>();
     protected Usage usage;
-    /** Used to see if the user edited the value. May be null. */
+    /** Used to see if the user edited the value. */
     protected String originalValue;
 
     /**
@@ -317 +318 @@
     /**
      * Returns the initial value to use for this preset.
      * <p>
-     * The initial value is the value shown in the control when the preset dialogs opens.
+     * The initial value is the value shown in the control when the preset dialog opens. For a
+     * discussion of all the options see the enclosed tickets.
      *
      * @param usage The key Usage
+     * @param support The support
      * @return The initial value to use.
+     *
+     * @see "https://josm.openstreetmap.de/ticket/5564"
+     * @see "https://josm.openstreetmap.de/ticket/12733"
+     * @see "https://josm.openstreetmap.de/ticket/17324"
      */
-    protected String getInitialValue(Usage usage) {
+    protected String getInitialValue(Usage usage, TaggingPresetItemGuiSupport support) {
         String initialValue = null;
-        originalValue = null;
+        originalValue = "";
 
         if (usage.hasUniqueValue()) {
             // all selected primitives have the same not empty value for this key
@@ -334 +341 @@
             // at least one primitive has a value for this key (but not all have the same one)
             initialValue = DIFFERENT;
             originalValue = initialValue;
-        } else if (PROP_FILL_DEFAULT.get() || isForceUseLastAsDefault()) {
+        } else if (!usage.hadKeys() || isForceUseLastAsDefault() || PROP_FILL_DEFAULT.get()) {
             // at this point no primitive had any value for this key
-            // use the last value no matter what
-            initialValue = LAST_VALUES.get(key);
-        } else if (!usage.hadKeys() && isUseLastAsDefault()) {
-            // use the last value only on objects with no keys at all
-            initialValue = LAST_VALUES.get(key);
-        } else if (!usage.hadKeys()) {
-            // use the default only on objects with no keys at all
-            initialValue = default_;
+            if (!support.isPresetInitiallyMatches() && isUseLastAsDefault() && LAST_VALUES.containsKey(key)) {
+                initialValue = LAST_VALUES.get(key);
+            } else {
+                initialValue = default_;
+            }
         }
-        return initialValue;
+        return initialValue != null ? initialValue : "";
     }
 
     @Override
@@ -353 +357 @@
         String value = getSelectedItem().value;
 
         // no change if same as before
-        if (value.isEmpty() && originalValue == null)
-            return;
         if (value.equals(originalValue))
             return;
         changedTags.add(new Tag(key, value));

src/org/openstreetmap/josm/gui/tagging/presets/items/MultiSelect.java

@@ -48 +48 @@
         model.clear();
         // disable if the selected primitives have different values
         list.setEnabled(usage.hasUniqueValue() || usage.unused());
-        String initialValue = getInitialValue(usage);
+        String initialValue = getInitialValue(usage, support);
 
         // Add values from the preset.
         presetListEntries.forEach(this::addEntry);
@@ -61 +61 @@
         }
 
         // Select the values in the initial value.
-        if (initialValue != null && !DIFFERENT.equals(initialValue)) {
+        if (!initialValue.isEmpty() && !DIFFERENT.equals(initialValue)) {
             for (String value : initialValue.split(String.valueOf(delimiter), -1)) {
                 PresetListEntry e = new PresetListEntry(value, this);
                 addEntry(e);

test/unit/org/openstreetmap/josm/gui/tagging/presets/items/ComboTest.java

@@ -27 +27 @@
      */
     @RegisterExtension
     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    public JOSMTestRules test = new JOSMTestRules().main().i18n("de");
+    public JOSMTestRules test = new JOSMTestRules().preferences().main().i18n("de");
 
     /**
      * Unit test for {@link Combo#addToPanel}.
@@ -54 +54 @@
         OsmPrimitive waySI = OsmUtils.createPrimitive("way addr:country=SI");
         KeyedItem.LAST_VALUES.clear();
         KeyedItem.LAST_VALUES.put("addr:country", "AT");
+        Combo.PROP_FILL_DEFAULT.put(false);
+        combo.use_last_as_default = 0;
 
-        combo.use_last_as_default = 0;
         combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, way));
         assertEquals("", combo.getSelectedItem().value);
         combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayTagged));
@@ -78 +79 @@
         assertEquals("SI", combo.getSelectedItem().value);
         combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT, waySI));
         assertEquals(Combo.DIFFERENT, combo.getSelectedItem().value);
+
+        Combo.PROP_FILL_DEFAULT.put(true);
+        combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, way));
+        assertEquals("AT", combo.getSelectedItem().value);
+        combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayTagged));
+        assertEquals("AT", combo.getSelectedItem().value);
+        combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT));
+        assertEquals("AT", combo.getSelectedItem().value);
+        combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, waySI));
+        assertEquals("SI", combo.getSelectedItem().value);
+        combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT, waySI));
+        assertEquals(Combo.DIFFERENT, combo.getSelectedItem().value);
+        Combo.PROP_FILL_DEFAULT.put(false);
         combo.default_ = null;
 
         combo.use_last_as_default = 1; // untagged objects only
@@ -103 +117 @@
         assertEquals("SI", combo.getSelectedItem().value);
         combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT, waySI));
         assertEquals(Combo.DIFFERENT, combo.getSelectedItem().value);
+        combo.use_last_as_default = 0;
 
         KeyedItem.LAST_VALUES.clear();
     }