RFR: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS [v2]

[Kevin Rushforth]

On Fri, 22 Jan 2021 12:02:13 GMT, Robert Lichtenberger <rlichten at openjdk.org> wrote:

>> By using the anchor location facility of PopupWindows we can avoid miscalculation of the
>> menu's height entirely.
>> This fix also cleans up some documentation issues.
>> This fix introduces tests that check the correct positioning (test_position_)
>> test_position_withCSS reproduces the problem that is fixed with this patch.
>> The other test_position_ cases serve as "proof" that no regressions are introduces.
>> They work before and after the fix is introduced.
>
> Robert Lichtenberger has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS
>   
>   Corrections as per Kevin Rushforth's comments.
>   Also added two more test cases that test right-to-left node orientation.
>   Fixed the implementation and the API documentation.

The fix and the tests look good to me. I have one comment on the docs (and I left a minor formatting comment).

modules/javafx.controls/src/main/java/javafx/scene/control/ContextMenu.java line 237:

> 235:      * NodeOrientation.RIGHT_TO_LEFT is set.
> 236:      * Using NodeOrientation.RIGHT_TO_LEFT will also "mirror" the meaning of Side.LEFT and
> 237:      * Side.RIGHT respectively.

We don't document the effect of node orientation in other controls or in charts, so I wouldn't want to mention it here. Instead you can document the behavior assuming the default effective orientation of `LEFT_TO_RIGHT` (without mentioning it).

You could make the case that we should document more precisely the effect or NodeOrientation, but that would be a large task, and not something I would want to do for an isolated control in the course of a bug fix (and it would require a CSR).

The rest of the doc changes look good and don't need a CSR.

modules/javafx.controls/src/main/java/javafx/scene/control/ContextMenu.java line 302:

> 300: 
> 301: 
> 302: 

Minor: there should only be one blank line here.

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

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