[Rev 01] RFR: 8244418: MenuBar: IOOB exception on requestFocus on empty bar

[Ajit Ghaisas]

On Fri, 8 May 2020 13:51:34 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review_fixes
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java line 485:
> 
>> 484:         }
>> 485:
>> 486:         if (focusedMenu != null && focusedMenuIndex != -1) {
> 
> don't quite understand why this change is necessary? The guard in the listener seems to be enough, at least the test
> passes without this. If it's needed to fix another potentially breaking path to this, would it be possible to add a
> test method that fails/passes before/after?

This was the place where exception was occurring. Hence, I added this check.
When I ran against the test, I still got the exception at caller lambda. I added that check later.
Wanted to remove this check - but simple code scan revealed this method gets called from engine.addTraverseListener()
listener with index = 0. Hence, I have kept this check. Covering this scenario in test seems tough.

> modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java line 307:
> 
>> 306:                 unSelectMenus();
>> 307:                 if (!container.getChildren().isEmpty()) {
>> 308:                     menuModeStart(0);
> 
> wondering whether this would be an appropriate time to simplify the logic a bit: as unSelectMenus is called in both if
> and else block, it could be condensed to
>     unSelectMenus();
>     if (t1 && !container.getChildren().isEmpty()) {
>        ...
>     }
> 
> might overlook something, though

Good suggestion. Fixed.

-------------

PR: https://git.openjdk.java.net/jfx/pull/216