Multiselect greyed out when the appropriate tag has a single known value

[gaben]

What steps will reproduce the problem?

1. Change JOSM locale to something other than English
2. Add a restaurant
3. Choose only one cuisine from the multiselect menu in the preset editor
4. Apply
5. Reopen the editor

What is the expected result?

The multiselect is usable.

What happens instead?

It's greyed out, not usable.

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

Regression from r18221.

Interestingly if you add two or more known tags, it works.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-09-12 03:30:18 +0200 (Sun, 12 Sep 2021)
Build-Date:2021-09-12 01:31:50
Revision:18218
Relative:URL: ^/trunk

Identification: JOSM/1.5 (18218 hu) Windows 10 64-Bit
OS Build number: Windows 10 Pro for Workstations 2009 (19043)
Memory Usage: 765 MB / 7266 MB (359 MB allocated, but free)
Java version: 1.8.0_301-b09, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920×1200 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1200
Best cursor sizes: 16×16→32×32, 32×32→32×32
System property file.encoding: Cp1250
System property sun.jnu.encoding: Cp1250
Locale info: hu_HU
Numbers with default locale: 1234567890 -> 1234567890
Dataset consistency test: No problems found

[gaben]

This seems to solve the issue: nope :(

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

@@ -35 +35 @@
         ComboMultiSelectListCellRenderer renderer = new ComboMultiSelectListCellRenderer(list, list.getCellRenderer(), 200, key);
         list.setCellRenderer(renderer);
         Object itemToSelect = getItemToSelect(def, support, true);
-        list.setSelectedItem(itemToSelect == null ? null : new PresetListEntry(itemToSelect.toString()));
+        list.setSelectedItem(new PresetListEntry(itemToSelect.toString()));
         JScrollPane sp = new JScrollPane(list);
         // if a number of rows has been specified in the preset,
         // modify preferred height of scroll pane to match that row count.

[skyper]

I cannot reproduce!

Only with an unknown value or leading/trailing white spaces the multiselect is disabled, for me, which did not change between r18193 and r18237.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-09-03 03:12:33 +0200 (Fri, 03 Sep 2021)
Revision:18193
Build-Date:2021-09-03 01:31:19
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (18193 en) Linux Debian GNU/Linux 11 (bullseye)
Memory Usage: 145 MB / 256 MB (58 MB allocated, but free)
Java version: 17-ea+19-Debian-1, Debian, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-09-30 01:35:56 +0200 (Thu, 30 Sep 2021)
Revision:18237
Build-Date:2021-09-30 01:31:00
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (18237 en) Linux Debian GNU/Linux 11 (bullseye)
Memory Usage: 144 MB / 256 MB (61 MB allocated, but free)
Java version: 17-ea+19-Debian-1, Debian, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel

[skyper]

The same behavior also with r18118. I do not find any regression.

[marcello@…]

Your steps to reproduce work as expected for me. Also the value written to the tag is legal.

But there is this comment in the source which implies that the behaviour observed is by design:

// check if we have actually managed to represent the full
// value with our presets. if not, cop out; we will not offer
// a selection list that threatens to ruin the value.
setEnabled(parts.isEmpty());

If you have a value in the tag that is not in the preset, then the multiselect will refuse to work.

We could fix this by simply adding the unknown value(s) to the multiselect and/or by shaving whitespace.

[gaben]

Is it Java related maybe? I use Java 8 across my machines.

Created a clean JOSM instance in a VM for testing and still reproducible there. Linux or Windows, doesn't matter.

[gaben]

Removed the extra parts

[skyper]

Replying to marcello@…:

We could fix this by simply adding the unknown value(s) to the multiselect and/or by shaving whitespace.

+1, for these enhancements

[GerdP]

I also cannot reproduce but I think there are various ways to open/apply presets. Maybe it depends on the usage of the mouse or shortcuts?
Example: Step 1 "Add a restaurant" means for me: Create a node, press Alt+A and start typing amenity until the word is shown, press Tab and start typing restaurant until the word is shown -> Enter -> Done.
I am pretty sure gaben means a different way. Same with the other steps.

[gaben]

The steps are missing a small part, otherwise fine. Locale matters! :D

When using non English locale, the new PresetListEntry(itemToSelect.toString()) returns the translated values. So when functions search for references, they can't find anything.
But when the object contains multiple values, it returns the original (untranslated) values and the matching functions work as intended.

[gaben]

added note about locale

[gaben]

typo

[skyper]

This is similar to #21336, I guess. Translated values seem to be a problem.

[gaben]

Restoring the pre r18221 behaviour fixes the issue. I didn't test further so please verify.

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

@@ -35 +35 @@
         ComboMultiSelectListCellRenderer renderer = new ComboMultiSelectListCellRenderer(list, list.getCellRenderer(), 200, key);
         list.setCellRenderer(renderer);
-        Object itemToSelect = getItemToSelect(def, support, true);
-        list.setSelectedItem(itemToSelect == null ? null : new PresetListEntry(itemToSelect.toString()));
+        list.setSelectedItem(getItemToSelect(def, support, true));
         JScrollPane sp = new JScrollPane(list);
         // if a number of rows has been specified in the preset,
         // modify preferred height of scroll pane to match that row count.
 

[marcello@…]

The problem is that getItemToSelect() returns too many different types when it should return String only or PresetListEntry only. I'll look into it.

[skyper]

I have a similar problem with default value set in preset: #21404.

[Don-vip]

In 18254/josm:

fix #21408 - fix #19013, fix #21385, fix #21404 - Fix MultiSelect issues (patch by marcello)

[gaben]

Hmm tested the patch, which fixed the issue but now I see an extra (1) in the cuisine field if I select a single cuisine, like this african (1). Is this intended?

[marcello@…]

Yes. It tells you how many of the selected items had that particular value set. I wanted to make it more like the Edit Tag dialog. It does not yet work with multiple values on one item though.

[skyper]

Replying to marcello@…:

Yes. It tells you how many of the selected items had that particular value set. I wanted to make it more like the Edit Tag dialog. It does not yet work with multiple values on one item though.

I thought about that, too, but I am not sure how this should work with multiple values. Right now I get the information that the number of objects only has this value and the rest has either multiple values or is missing the key.

[gaben]

Ooh, nice touch! :D

[Don-vip]

Milestone renamed