On Sun, 21 Dec 2025 18:03:27 GMT, Cormac Redmond <[email protected]> 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
