On Mon, 21 Sep 2026 12:22:26 GMT, Ziad El Midaoui <[email protected]> 
wrote:

>> Improved the manual test instructions and pass/fail criteria for the 
>> following tests :
>> 
>> - NotResizableWindowTest
>> - DndBasic
>> - DndTestDragViewRawImage
>> - PrintDialogModalityTest
>> - PrintOrientTest
>> - StartIconified
>> - DragDropFromSwingComponentInSwingNodeTest
>> - DragDropOntoJavaFXControlInJFXPanelTest
>> - EmojiTest
>> - EventListenerLeak
>> - InputTypeAcceptAttributeTest
>> - GifImageTestApp
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Ziad El Midaoui has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Minor : Trailing whitespaces removed

I left a few comments about the alignment of text block. I'll approve it as-is 
and reapprove if you make any changes.

tests/manual/dnd/DndBasic.java line 103:

> 101:             modifiers =
> 102:                     "macOS: Use no modifier for COPY, 'Command' for 
> MOVE, " +
> 103:                             "and 'Control+Option' for LINK.";

On my macOS 15 system, netierh CMD+Option nor Control+Option will enable LINK. 
I tested this by dragging from the top box on the left to the top box on the 
right. The "Move" keyboard modifier works, but the "Link" doesn't work for me.

tests/manual/stage/StartIconified.java line 66:

> 64: 
> 65:         Text text = new Text("""
> 66:                     This stage must initially appear on the OS taskbar 
> (iconified), but not on the Screen.

Minor: I would align this with the closing `"""`

tests/manual/text/EmojiTest.java line 43:

> 41:     static String instructions =
> 42:             """
> 43:                     This tests rendering of Emoji glyphs, which is only 
> supported on macOS.

Minor: I see no need for 8 more spaces of indentation here. I recommend to 
align the rest of the lines with the opening `"""`.

tests/manual/web/EventListenerLeak.java line 251:

> 249:         VBox instructions = new VBox(
> 250:                 new Label("""
> 251:                          This test is for EventListener memory leak 
> manual testing\s

I realize this was preexisting, but the extra space before "This" seems odd. It 
shows up in the Label (as it did before your change) and makes the alignment 
look off. In fact, the use of an extra space on most, but not all, lines is not 
the best way to do it. This would be OK to fix in the follow-on issue if you 
prefer.

tests/manual/web/InputTypeAcceptAttributeTest.java line 57:

> 55:         VBox instructions = new VBox(
> 56:                 new Label("""
> 57:                          This test creates four files (TEXT.txt, PNG.png, 
> PDF.pdf, JPG.jpg) at below mentioned path:

Similar to EventListenerLeak, this uses spaces to try to add padding. OK for 
this PR, but you might consider fixing it in a follow-up.

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

Marked as reviewed by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/2315#pullrequestreview-5269180601
PR Review Comment: https://git.openjdk.org/jfx/pull/2315#discussion_r4064265063
PR Review Comment: https://git.openjdk.org/jfx/pull/2315#discussion_r4064413441
PR Review Comment: https://git.openjdk.org/jfx/pull/2315#discussion_r4064205874
PR Review Comment: https://git.openjdk.org/jfx/pull/2315#discussion_r4064372264
PR Review Comment: https://git.openjdk.org/jfx/pull/2315#discussion_r4064393308

Reply via email to