Presets menu: Sorting fails on second and third item (Search actions should be on top)

[skyper]

What steps will reproduce the problem?

1. Start JOSM with fresh preferences
2. Look at the order of the first three items of the presets menu

What is the expected result?

Both search actions on top and preferences below

What happens instead?

Preferences is the second item in between the two search actions

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

I noticed this while updating the documentation wiki:Help/Menu/Presets. The screenshot shows the correct order and with my common preferences the order is the same as the screenshot. Over the years somethings has changed the order but I do not know the change to blame.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2023-03-02 08:22:33 +0100 (Thu, 02 Mar 2023)
Revision:18679
Build-Date:2023-03-03 02:30:57
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (18679 en) Linux Debian GNU/Linux 11 (bullseye)
Memory Usage: 167 MB / 256 MB (66 MB allocated, but free)
Java version: 17.0.6+10-Debian-1deb11u1, Debian, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel
Desktop environment: GNOME
VM arguments: [-Djosm.home=<josm.pref>]

[skyper]

Maybe, #22784 can be fixed along side.

[gaben]

Interesting, there is a taggingpreset.sortvalues advanced preference, if you set it to false, the order becomes correct.

Relevant code here: source:trunk/src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresets.java?rev=18363#L126​

[gaben]

With enabled/disabled sorting

[skyper]

Interesting so the order is correct in the source code but sorting fails.

[taylor.smock]

I've got a fix for this. I'm working on a non-regression test for it right now.

[gaben]

Thanks for the heads up, just fired the dev VM to inspect further :D

In that case I'm curios what will be the outcome.

[taylor.smock]

The fix was really easy -- the problem was that we are sorting every object in the menu except for the first Search preset... action. See source:trunk/src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetMenu.java​ (look at the inner class PresetTextComparator).

The test I just finished writing checks against both the sorted menu and the non-sorted menu, checking the order of the preset search action, the preset object search action, the preferences action, and the separator. This is mostly to ensure if someone decides to add an additional action, it too will get properly sorted once they update the test and comparator.

[taylor.smock]

In 18683/josm:

Fix #22783: Presets menu should not sort the first action block

The specific problem was that when the preset menu was sorted, the order of
the Preset menu (prior to the first separator) was Search preset...,
Preset preferences..., Search for objects by preset... instead of
Search preset..., Search for objects by preset..., Preset Preferences...

The fix for this was changing the TaggingPresetMenu.PresetTextComparator.compare
method to have the same behavior for the ordered presets. A non-regression
test was also added.

[gaben]

Oh god, thanks! I haven't checked what it compares to...

I was thinking about skipping the sort until the first separator/preset group to have the flexibility, but that's fine, I'm not expecting a change of the order anytime soon.

[taylor.smock]

Replying to gaben:

Oh god, thanks! I haven't checked what it compares to...

I was thinking about skipping the sort until the first separator/preset group to have the flexibility, but that's fine, I'm not expecting a change of the order anytime soon.

That was actually my first instinct. The problem with that is it recursively calls itself (take a look at L145). We could technically have it call another method with a parameter for skipping <x> number of separators.