changeset 9809:838868f00d35

8090201: Keyboard traversal of menu with hidden menu item is broken contributed-by: rlichten reviewed-by: jgiles
author jgiles
date Tue, 24 May 2016 11:17:32 +1200
parents b603693ce9c2
children c896deb35f10
files modules/controls/src/main/java/com/sun/javafx/scene/control/ContextMenuContent.java modules/controls/src/test/java/test/javafx/scene/control/ContextMenuTest.java
diffstat 2 files changed, 33 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/ContextMenuContent.java	Sat May 21 07:06:57 2016 -0700
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/ContextMenuContent.java	Tue May 24 11:17:32 2016 +1200
@@ -692,14 +692,14 @@
     private int findNext(int from) {
         for (int i = from; i < itemsContainer.getChildren().size(); i++) {
             Node n = itemsContainer.getChildren().get(i);
-            if (n instanceof MenuItemContainer) {
+            if (n instanceof MenuItemContainer && n.isVisible()) {
                 return i;
             }
         }
         // find from top
         for (int i = 0; i < from; i++) {
             Node n = itemsContainer.getChildren().get(i);
-            if (n instanceof MenuItemContainer) {
+            if (n instanceof MenuItemContainer && n.isVisible()) {
                 return i;
             }
         }
@@ -731,13 +731,13 @@
     private int findPrevious(int from) {
         for (int i = from; i >= 0; i--) {
             Node n = itemsContainer.getChildren().get(i);
-            if (n instanceof MenuItemContainer) {
+            if (n instanceof MenuItemContainer && n.isVisible()) {
                 return(i);
             }
         }
         for (int i = itemsContainer.getChildren().size() - 1 ; i > from; i--) {
             Node n = itemsContainer.getChildren().get(i);
-            if (n instanceof MenuItemContainer) {
+            if (n instanceof MenuItemContainer && n.isVisible()) {
                 return(i);
             }
         }
--- a/modules/controls/src/test/java/test/javafx/scene/control/ContextMenuTest.java	Sat May 21 07:06:57 2016 -0700
+++ b/modules/controls/src/test/java/test/javafx/scene/control/ContextMenuTest.java	Tue May 24 11:17:32 2016 +1200
@@ -118,10 +118,10 @@
 
     @Before public void setup() {
         // earlier test items
-        menuItem0 = new MenuItem();
-        menuItem1 = new MenuItem();
-        menuItem2 = new MenuItem();
-        menuItem3 = new MenuItem();
+        menuItem0 = new MenuItem("0");
+        menuItem1 = new MenuItem("1");
+        menuItem2 = new MenuItem("2");
+        menuItem3 = new MenuItem("3");
 
         contextMenu = new ContextMenu();
         contextMenuWithOneItem = new ContextMenu(menuItem0);
@@ -550,4 +550,29 @@
         assertEquals(subMenu, getShowingSubMenu(cm));
         assertEquals(subMenuItem1, getCurrentFocusedItem(cm));
     }
+
+    @Test public void test_navigateMenu_withInvisibleItems_rt40689() {
+        ContextMenu cm = contextMenuWithManyItems;
+        cm.show(anchorBtn, Side.RIGHT, 0, 0);
+
+        menuItem2.setVisible(false);
+
+        assertNotNull(getShowingMenuContent(cm));
+        assertEquals(-1, getCurrentFocusedIndex(cm));
+
+        // press down once to go to menuItem
+        pressDownKey(cm);
+        MenuItem focusedItem = getCurrentFocusedItem(cm);
+        assertEquals("Expected " + menuItem1.getText() + ", found " + focusedItem.getText(), menuItem1, focusedItem);
+
+        // press down again should skip invisible menuItem2 and proceed to menuItem3
+        pressDownKey(cm);
+        focusedItem = getCurrentFocusedItem(cm);
+        assertEquals("Expected " + menuItem3.getText() + ", found " + focusedItem.getText(), menuItem3, focusedItem);
+
+        // press up should skip invisible menuItem2 and proceed to menuItem1
+        pressUpKey(cm);
+        focusedItem = getCurrentFocusedItem(cm);
+        assertEquals("Expected " + menuItem1.getText() + ", found " + focusedItem.getText(), menuItem1, focusedItem);
+    }
 }