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

[Jeanette Winzenburg]

On Fri, 12 Jun 2020 21:17:09 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review fixes and test addition
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java line 764:
> 
>> 763:     void setFocusedIndex(int index) {
>> 764:         this.setFocusedMenuIndex(0);
>> 765:     }
> 
> Shouldn't this be `this.setFocusedMenuIndex(index)`? The only reason this isn't causing problems is that the test you
> added never calls it with anything other than 0.
> Also, the naming of this method is a bit confusing. I might recommend calling it `test_setFocusedMenuIndex`, or else
> just change the private `setFocusedMenuIndex` method to be package-scope. Either way adding a comment indicating that
> it is used for testing purposes seems a good idea (if you keep this method then the comment could say that it is only
> for testing purposes; if you make `setFocusedMenuIndex` package-scope then you could say that it is package scope
> because it is used for testing).

darn .. overlooked that fixed index ..

As to the naming: personally, I hate violation of naming conventions even for test-only methods, so would tend to make
setFocusedMenuIndex  package and doc as being used for testing as well as internally.

We might consider aligning the corresponding methods in the shim (they are also slightly confusing in
get/set/FocusedIndex).

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

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