Changeset 18586 in josm

Timestamp:

2022-11-07T21:46:03+01:00 (2 years ago)

Author:

taylor.smock

Message:

Fix #22487: Object hover preview is re-enabled on layer change even when disabled in properties (patch by Woazboat, modified)

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialog.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialogTest.java (modified) (3 diffs)

trunk/src/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialog.java

@@ -895 +895 @@
         if (newLayer != null) {
             newLayer.addPropertyChangeListener(this);
-            if (newLayer.isVisible()) {
+            if (newLayer.isVisible() && Boolean.TRUE.equals(PROP_PREVIEW_ON_HOVER.get())) {
                 MainApplication.getMap().mapView.addPrimitiveHoverListener(this);
             } else {
@@ -909 +909 @@
 
             // Disable hover preview when primitives are invisible
-            if (isVisible) {
+            if (isVisible && Boolean.TRUE.equals(PROP_PREVIEW_ON_HOVER.get())) {
                 MainApplication.getMap().mapView.addPrimitiveHoverListener(this);
             } else {

trunk/test/unit/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialogTest.java

@@ -3 +3 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.stream.Stream;
 
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.openstreetmap.josm.data.coor.LatLon;
+import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.NavigatableComponent;
+import org.openstreetmap.josm.gui.PrimitiveHoverListener;
+import org.openstreetmap.josm.gui.layer.OsmDataLayer;
+import org.openstreetmap.josm.testutils.JOSMTestRules;
 import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
-
-import org.junit.jupiter.api.Test;
+import org.openstreetmap.josm.tools.ReflectionUtils;
 
 /**
@@ -20 +35 @@
 @BasicPreferences
 class PropertiesDialogTest {
+    @RegisterExtension
+    static JOSMTestRules rules = new JOSMTestRules().main().projection();
+
     private static String createSearchSetting(List<OsmPrimitive> sel, boolean sameType) {
         return PropertiesDialog.createSearchSetting("foo", sel, sameType).text;
@@ -54 +72 @@
         assertEquals("(type:way \"foo\"=\"bar\") OR (type:node \"foo\"=\"bar\")", createSearchSetting(sel, true));
     }
+
+    static Stream<Arguments> testTicket22487() {
+        return Stream.of(
+                Arguments.of("Layer add", (Runnable) () ->
+                        MainApplication.getLayerManager().addLayer(new OsmDataLayer(new DataSet(), "testTicket22487-layerAdd", null))),
+                Arguments.of("Layer hide", (Runnable) () -> {
+                    // We need to toggle the layer visibility to hit the bug.
+                    MainApplication.getLayerManager().getLayers().forEach(layer -> layer.setVisible(false));
+                    MainApplication.getLayerManager().getLayers().forEach(layer -> layer.setVisible(true));
+                })
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource
+    void testTicket22487(String ignored_title, Runnable action) throws ReflectiveOperationException {
+        Field primitiveHoverListenersField = NavigatableComponent.class.getDeclaredField("primitiveHoverListeners");
+        ReflectionUtils.setObjectsAccessible(primitiveHoverListenersField);
+
+        DataSet ds = new DataSet();
+        OsmDataLayer layer = new OsmDataLayer(ds, "testTicket22487", null);
+        // Ensure that the navigatable component is set up
+        MainApplication.getLayerManager().addLayer(layer);
+        PropertiesDialog propertiesDialog = MainApplication.getMap().propertiesDialog;
+        @SuppressWarnings("unchecked")
+        CopyOnWriteArrayList<PrimitiveHoverListener> listeners =
+                (CopyOnWriteArrayList<PrimitiveHoverListener>) primitiveHoverListenersField.get(MainApplication.getMap().mapView);
+        assertTrue(listeners.contains(propertiesDialog));
+
+        // Set the properties to false
+        PropertiesDialog.PROP_PREVIEW_ON_HOVER.put(false);
+        assertFalse(listeners.contains(propertiesDialog));
+
+        action.run();
+        assertFalse(listeners.contains(propertiesDialog));
+    }
 }