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

[Robert Lichtenberger]

On Thu, 21 Jan 2021 23:07:23 GMT, Kevin Rushforth <kcr 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.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/ContextMenu.java line 241:
> 
>> 239:      * the {@code ContextMenu} such that its top-left (0,0) position would be attached
>> 240:      * to the top-right position of the anchor.
>> 241:      * <p>
> 
> I think it is worth adding a short replacement for this, explaining how the side parameter and delta values affect the location of the popup.

While trying to come up with a good documentation I've detected a real change in behaviour in connection with the NodeOrientation of the anchor node.
Although this has never been documented, when NodeOrientation.RIGHT_TO_LEFT is set, the context menu was right-aligned for Side=TOP and Side=BOTTOM. 
Since I assume we don't want to change this behaviour I would now document it and adapt my patch accordingly. I've already written a test case like this:
    @Test public void test_position_withOrientation() throws InterruptedException {
        ContextMenu cm = createContextMenu(false);
        anchorBtn.setNodeOrientation(NodeOrientation.RIGHT_TO_LEFT);
        cm.show(anchorBtn, Side.TOP, 0, 0);

        Bounds anchorBounds = anchorBtn.localToScreen(anchorBtn.getLayoutBounds());
        Node cmNode = cm.getScene().getRoot();
        Bounds cmBounds = cm.getScene().getRoot().localToScreen(cmNode.getLayoutBounds());

        assertEquals(anchorBounds.getMaxX(), cmBounds.getMaxX(), 0.0);
        assertEquals(anchorBounds.getMinY(), cmBounds.getMaxY(), 0.0);
        anchorBtn.setNodeOrientation(NodeOrientation.LEFT_TO_RIGHT);
    }
which passes with the old implementation but (currently) fails with the new implementation.

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

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