RFR: 8366739: ToolBar: overflow menu with fractional scale (2)
Marius Hanl
mhanl at openjdk.org
Sun Dec 21 21:02:19 UTC 2025
On Sun, 21 Dec 2025 18:03:27 GMT, Cormac Redmond <duke at openjdk.org> wrote:
>> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/ToolBarSkinTest.java line 113:
>>
>>> 111:
>>> 112: BorderPane bp = new BorderPane();
>>> 113: bp.setTop(new HBox(toolBar));
>>
>> Is it possible to use the `HBox` directly, like the other test?
>> The `BorderPane` setup is fine, I'm just asking since if we don't need it to reproduce it, we can also remove it to minify the test setup.
>
> Yes and no.
>
> Using a BorderPane was intentional. The BorderPane **was** always necessary to cause this for HORIZONTAL orientation, I remember noting it when originally filing the bug (before knowing what the cause was).
>
> Oddly, something must have changed in recent versions (there is some chatter about changes related to a previous fix), because now it's not necessary; the bug will happen with or without BorderPane for HORIZONTAL.
>
> However, to reproduce the error for VERTICAL orientation, the BorderPane is still required (why exactly, I'm not sure).
>
> In hindsight, it must just affect whatever way the numbers work out. A bit of luck (or bad luck).
>
> For VERTICAL, if you use a HBox directly (and remove the fix), you'll notice the VERTICAL orientation test still passes (i.e., no bug). But if you add the BorderPane, you'll notice the bug occurs.
>
> So it's "required" to reproduce the bug there... of course, it could be reproduced probably in many ways.
>
> One of the reasons I refactored assertOverflowNotShown() slightly was so the test can hand in any type of scene setup it wants, complex or simple. The problem here is the fix is simple, reproducing is hard. In fact, with my tests / scene setup, the bug only happens at scaling 1.25 scaling. Not 1.75; didn't really investigate why as it's going to boil down to just rounding luck.
Thanks. This sounds like a good reason why should keep `BorderPane`.
> Not 1.75; didn't really investigate why as it's going to boil down to just rounding luck.
Yes, I can confirm that as well as I also did a bunch of snapping fixes in the past.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2016#discussion_r2638042557
More information about the openjfx-dev
mailing list