On Tue, 12 May 2020 12:27:11 GMT, Ajit Ghaisas <aghai...@openjdk.org> wrote:
>> Issue : >> https://bugs.openjdk.java.net/browse/JDK-8244418 >> >> Root Cause : >> Incorrect assumption about menu list size. >> >> Fix : >> Added check for empty menu list before trying to access it. >> >> Test : >> Added a unit test that fails before fix and passes after it. > > Ajit Ghaisas has updated the pull request incrementally with one additional > commit since the last revision: > > review_fixes modules/javafx.controls/src/test/java/test/javafx/scene/control/MenuBarTest.java line 134: > 133: int focusedIndex = MenuBarSkinShim.getFocusedIndex(skin); > 134: assertEquals(focusedIndex, -1); > 135: } the assert should be the other way round, expected value should be the first parameter :) didn't notice on first read and now only when writing a test case against our argument, c&p'ed the test as-is, replaced the requestFocus with simulating the notification from traversalListener and was confused about its value being -1 Here's that modified test method (requires test api in MenuBarSkin and shim): @Test public void testSimulateTraverseIntoOnEmptyMenubar() { menuBar.setFocusTraversable(true); AnchorPane root = new AnchorPane(); root.getChildren().add(menuBar); startApp(root); MenuBarSkin skin = (MenuBarSkin)menuBar.getSkin(); assertTrue(skin != null); // simulate notification from traversalListener MenuBarSkinShim.setFocusedIndex(skin, 0); int focusedIndex = MenuBarSkinShim.getFocusedIndex(skin); assertEquals(-1, focusedIndex); } ------------- PR: https://git.openjdk.java.net/jfx/pull/216
