[patch] Search button's dropdown list not initialized on JOSM lunch

[gaben]

What steps will reproduce the problem?

1. Lunch JOSM
2. Have some search history in the preferences by making some search, content doesn't matter
3. Relaunch JOSM
4. Add a new empty layer
5. Click on the arrow button

What is the expected result?

The dropdown list is available from lunch, without clicking on the Search button.

What happens instead?

One need to click Search button first to initialise the search history, then the dropdown list will be available.

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

It makes some of the manual dev testing hard, because of the extra clicks.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2023-04-27 16:45:28 +0200 (Thu, 27 Apr 2023)
Build-Date:2023-04-28 01:30:56
Revision:18719
Relative:URL: ^/trunk

Identification: JOSM/1.5 (18719 hu) Windows 10 64-Bit
OS Build number: Windows 10 Pro for Workstations 2009 (19045)
Memory Usage: 975 MB / 7266 MB (611 MB allocated, but free)
Java version: 1.8.0_371-b11, 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
VM arguments: [-Djava.security.manager, -Djnlp.application.href=https://josm.openstreetmap.de/download/josm-latest.jnlp, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlp.tk=awt, -Djnlpx.jvm=<java.home>\bin\javaw.exe, -Djnlpx.splashport=58330, -Djnlpx.home=<java.home>\bin, -Djnlpx.remove=false, -Djnlpx.offline=false, -Djnlpx.relaunch=true, -Djnlpx.session.data=%UserProfile%\AppData\Local\Temp\session6912936828087324594, -Djnlpx.heapsize=NULL,NULL, -Djava.security.policy=file:<java.home>\lib\security\javaws.policy, -DtrustProxy=true, -Djnlpx.origFilenameArg=%UserProfile%\AppData\LocalLow\Sun\Java\Deployment\cache\6.0\31\583aa85f-32780bce]

[gaben]

Maybe it fits in April's release as it's a minor change :)

[taylor.smock]

Ticket retargeted after milestone closed

[gaben]

in javadoc use @link instead of @code

[taylor.smock]

As a stupid question, why not in the static initializer? Like so:

src/org/openstreetmap/josm/actions/search/SearchAction.java

@@ -72 +72 @@
             SearchSetting::readFromString, SearchSetting::writeToString);
 
     static {
+        // Load the history on initial load (for the drop-down dialog)
+        prefs.load("search.history");
         SearchCompiler.addMatchFactory(new SimpleMatchFactory() {
             @Override
             public Collection<String> getKeywords() {

[gaben]

It's a good one, I completely missed the static initializer.

In that case the preference load call can be moved from the search() method to the initializer, although I'm not sure why. I suspect the AutoCompComboBoxModel.Preferences gets associated with the given preference key, which is needed only once.

src/org/openstreetmap/josm/actions/search/SearchAction.java

@@ -72 +72 @@
             SearchSetting::readFromString, SearchSetting::writeToString);
 
     static {
+        // Load the history on initial load (for the drop-down dialog)
+        prefs.load("search.history");
         SearchCompiler.addMatchFactory(new SimpleMatchFactory() {
             @Override
             public Collection<String> getKeywords() {
@@ -185 +186 @@
      * Launches the dialog for specifying search criteria and runs a search
      */
     public static void search() {
-        prefs.load("search.history");
         SearchSetting se = showSearchDialog(lastSearch);
         if (se != null) {
             searchWithHistory(se);

[taylor.smock]

I didn't move it out of the search function just in case someone, somewhere, is fiddling with the search.history config variable. The combobox doesn't actually add a listener on the config key, so changes won't propagate to the search dialog.

It probably worked for you in testing since prefs will add the search term to the model and then save the preferences value. Funky things would start happening if a plugin decides to allow users to use the search terms from the search dialog and then said plugin saves new search terms to the same preference key.

I think it would be better to just leave the prefs.load("search.history") in the search function.

[gaben]

Ah yes, we need to think about plugins as well. Then it's clear.

[taylor.smock]

In 18732/josm:

Fix #22908: Search button's dropdown list not initialized on JOSM launch (patch by gaben, modified)