On Tue, 8 Sep 2026 20:04:40 GMT, Andy Goryachev <[email protected]> wrote:
>> Marius Hanl has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Use String.join
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/StyleManager.java
> line 1638:
>
>> 1636:
>> 1637: final String styleClass = styleClasses.get(n);
>> 1638: if (styleClass == null || styleClass.isEmpty())
>> continue;
>
> please use curly braces and place continue on its own line
note that this is the same as before. But changed as the diff is showing it
anyway.
> modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java line
> 112:
>
>> 110: public void setUp() {
>> 111: toolkit = (StubToolkit) Toolkit.getToolkit();
>> 112: stage = new Stage();
>
> 1. many other tests do `((StubToolkit) Toolkit.getToolkit())` so it probably
> makes sense either do the same, or fix the other test to use this reference
> 2. this Stage is being created for each test, but only used it in one, is
> this right?
>
> what do you think?
changed. Regarding `StubToolkit`, normally the cast is not needed. But
`StubToolkit` has some methods that might be needed (rarely) for tests.
Change the `Stage` logic. Note that the `StageLoader` would be perfect here,
but only accessible in `javafx.controls`. I would really like to move it in a
testbug PR, but the diff will probably be huge. What do you think? If this is
okay, I will create a ticket.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2191#discussion_r3971847269
PR Review Comment: https://git.openjdk.org/jfx/pull/2191#discussion_r3971843378
