Modify

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#21385 closed defect (fixed)

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

Reported by: gaben Owned by: team
Priority: normal Milestone: 21.10
Component: Core Version:
Keywords: template_report regression multiselect Cc:

Description (last modified by 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

Attachments (0)

Change History (24)

comment:1 by gaben, 2 years ago

Description: modified (diff)

comment:2 by gaben, 2 years ago

This seems to solve the issue: nope :(

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

    a b  
    3535        ComboMultiSelectListCellRenderer renderer = new ComboMultiSelectListCellRenderer(list, list.getCellRenderer(), 200, key);
    3636        list.setCellRenderer(renderer);
    3737        Object itemToSelect = getItemToSelect(def, support, true);
    38         list.setSelectedItem(itemToSelect == null ? null : new PresetListEntry(itemToSelect.toString()));
     38        list.setSelectedItem(new PresetListEntry(itemToSelect.toString()));
    3939        JScrollPane sp = new JScrollPane(list);
    4040        // if a number of rows has been specified in the preset,
    4141        // modify preferred height of scroll pane to match that row count.
Last edited 2 years ago by gaben (previous) (diff)

comment:3 by gaben, 2 years ago

Keywords: multiselect added

comment:4 by skyper, 2 years ago

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
Last edited 2 years ago by skyper (previous) (diff)

comment:5 by skyper, 2 years ago

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

comment:6 by marcello@…, 2 years ago

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.

comment:7 by gaben, 2 years ago

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.

in reply to:  6 comment:8 by gaben, 2 years ago

Description: modified (diff)
Summary: Multiselect greyed out when the appropriate tag has a single valueMultiselect greyed out when the appropriate tag has a single known value

Removed the extra parts

comment:9 by Don-vip, 2 years ago

Milestone: 21.09

in reply to:  6 comment:10 by skyper, 2 years ago

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

comment:11 by GerdP, 2 years ago

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.

comment:12 by gaben, 2 years ago

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.

comment:13 by gaben, 2 years ago

Description: modified (diff)

added note about locale

comment:14 by gaben, 2 years ago

Description: modified (diff)

typo

comment:15 by skyper, 2 years ago

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

comment:16 by gaben, 2 years ago

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

    a b  
    3535        ComboMultiSelectListCellRenderer renderer = new ComboMultiSelectListCellRenderer(list, list.getCellRenderer(), 200, key);
    3636        list.setCellRenderer(renderer);
    37         Object itemToSelect = getItemToSelect(def, support, true);
    38         list.setSelectedItem(itemToSelect == null ? null : new PresetListEntry(itemToSelect.toString()));
     37        list.setSelectedItem(getItemToSelect(def, support, true));
    3938        JScrollPane sp = new JScrollPane(list);
    4039        // if a number of rows has been specified in the preset,
    4140        // modify preferred height of scroll pane to match that row count.
    4241
Last edited 2 years ago by gaben (previous) (diff)

comment:17 by marcello@…, 2 years ago

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

comment:18 by skyper, 2 years ago

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

comment:19 by Don-vip, 2 years ago

Resolution: fixed
Status: newclosed

In 18254/josm:

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

comment:20 by gaben, 2 years ago

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?

comment:21 by marcello@…, 2 years ago

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.

in reply to:  21 comment:22 by skyper, 2 years ago

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.

comment:23 by gaben, 2 years ago

Ooh, nice touch! :D

comment:24 by Don-vip, 2 years ago

Milestone: 21.0921.10

Milestone renamed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.