On Sun, 21 Dec 2025 17:41:03 GMT, Marius Hanl <[email protected]> wrote:
>> Fix overflow menu triggering due to floating-point precision error. >> >> At 1.25 display scaling on Windows, floating-point comparison errors (e.g. >> 109.60000000000001 > 109.6) cause the overflow menu to appear when the >> lefthand value is regarded as larger than the righthand value. >> >> These should be treated as equal (and therefore not display the overflow >> menu). >> >> This bug can happen in both horizontal and vertical toolbar orientation. >> >> The new tests added fail without this fix, and pass with it. An existing >> test has been re-factored slightly to allow re-use and more flexibility in >> specifying the scene's root node. > > 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. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2016#discussion_r2638003182
