8297130: ComboBox popup doesn't close after selecting value that was …

…added with 'runLater'

Reviewed-by: angorya, aghaisas

modules/javafx.graphics/src/main/java/javafx/scene/Node.java

@@ -957,7 +957,7 @@ private ReadOnlyObjectWrapper<Parent> parentPropertyImpl() {
                 @Override
                 protected void invalidated() {
                     if (oldParent != null) {
-                        updateRemovedParentFocus(oldParent);
+                        clearParentsFocusWithin(oldParent);
                         oldParent.disabledProperty().removeListener(parentDisabledChangedListener);
                         oldParent.treeVisibleProperty().removeListener(parentTreeVisibleChangedListener);
                         if (nodeTransformation != null && nodeTransformation.listenerReasons > 0) {
@@ -8184,15 +8184,16 @@ final void notifyFocusListeners() {
     }
 
     /**
-     * Called when the current node was removed from the scene graph in order to clear
-     * the focus bits of the former parents.
+     * Called when the current node was removed from the scene graph.
+     * If the current node has the focusWithin bit, we also need to clear the focusWithin bits of this
+     * node's parents. Note that a scene graph can have more than a single focused node, for example when
+     * a PopupWindow is used to present a branch of the scene graph. Since we need to preserve multi-level
+     * focus, we need to stop clearing the focusWithin bits if we encounter a parent node that is focused.
      */
-    private void updateRemovedParentFocus(Node oldParent) {
+    private void clearParentsFocusWithin(Node oldParent) {
         if (oldParent != null && focusWithin.get()) {
             Node node = oldParent;
-            while (node != null) {
-                node.focused.set(false);
-                node.focusVisible.set(false);
+            while (node != null && !node.focused.get()) {
                 node.focusWithin.set(false);
                 node = node.getParent();
             }
@@ -8239,7 +8240,7 @@ public void set(boolean value) {
                 do {
                     node.focusWithin.set(value);
                     node = node.getParent();
-                } while (node != null);
+                } while (node != null && !node.focused.get());
             }
         }
     };

modules/javafx.graphics/src/test/java/test/javafx/scene/FocusTest.java

@@ -117,18 +117,26 @@ Node n() {
         return n(T, T, T);
     }
 
-    private void assertIsFocused(Scene s, Node n) {
-        assertEquals(n, s.getFocusOwner());
+    private void assertIsFocused(Node n) {
         assertTrue(n.isFocused());
         assertTrue(n.getPseudoClassStates().stream().anyMatch(pc -> pc.getPseudoClassName().equals("focused")));
     }
 
-    private void assertNotFocused(Scene s, Node n) {
-        assertTrue(n != s.getFocusOwner());
+    private void assertIsFocused(Scene s, Node n) {
+        assertEquals(n, s.getFocusOwner());
+        assertIsFocused(n);
+    }
+
+    private void assertNotFocused(Node n) {
         assertFalse(n.isFocused());
         assertFalse(n.getPseudoClassStates().stream().anyMatch(pc -> pc.getPseudoClassName().equals("focused")));
     }
 
+    private void assertNotFocused(Scene s, Node n) {
+        assertTrue(n != s.getFocusOwner());
+        assertNotFocused(n);
+    }
+
     private void assertNullFocus(Scene s) {
         assertNull(s.getFocusOwner());
     }
@@ -998,4 +1006,49 @@ private void fireTouchPressedEvent(EventTarget target) {
         assertNotFocusWithin(g2);
     }
 
+    /**
+     * When a scene graph contains multiple nested focused nodes, the focusWithin bits that are
+     * cleared when a focused node is removed must only be cleared as long as we don't encounter
+     * another focused node up the tree.
+     */
+    @Test public void testMultiLevelFocusWithinIsPreserved() {
+        class N extends Group {
+            N(Node... children) {
+                super(children);
+                setFocusTraversable(true);
+            }
+
+            void setFocused() {
+                setFocused(true);
+            }
+        }
+
+        N node1, node2, node3, node4;
+
+        scene.setRoot(
+            node1 = new N(
+                node2 = new N(
+                    node3 = new N(
+                        node4 = new N()
+                    )
+                )
+            ));
+
+        node2.setFocused();
+        node4.setFocused();
+
+        // Remove node4 from the scene graph
+        node3.getChildren().clear();
+
+        assertIsFocusWithin(node1);
+        assertIsFocusWithin(node2);
+        assertNotFocusWithin(node3);
+        assertIsFocusWithin(node4);
+
+        assertNotFocused(node1);
+        assertIsFocused(node2);
+        assertNotFocused(node3);
+        assertIsFocused(node4);
+    }
+
 }