[Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings

[Kevin Rushforth]

On Tue, 17 Dec 2019 21:43:32 GMT, Thiago Milczarek Sayao <tsayao at openjdk.org> wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8232811
>> 
>> This one was hard to tackle.
>> 
>> ./gradlew -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test --tests test.robot.javafx.scene.dialog.DialogWithOwnerSizingTest
> 
> The pull request has been updated with 1 additional commit.

The fix looks good (with a minor formatting comment). The test needs a couple of changes.

modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1053:

> 1052:         return;
> 1053: 
> 1054:     }

Minor: This blank line is not related to the change; probably best to remove it.

modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1289:

> 1288:         }
> 1289: 
> 1290:         gtk_window_resize(GTK_WINDOW(gtk_widget), newWidth, newHeight);

Ditto.

tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java line 63:

> 62:     @Test(timeout = 15000)
> 63:     public void dialogWithOwnerSizingTest() throws Exception {
> 64:         Thread.sleep(500);

This test fails on Windows 10 with 125% screen scaling. The size of the two dialogs are not exactly the same: 181.6 vs 180.0. I recommend either excluding the test or else adding a 2 pixel tolerance if the DPI scaling factor is non-integral.

tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java line 68:

> 67:         Assert.assertEquals(dialog.getDialogPane().getWidth(), dialog2.getDialogPane().getWidth(), 0.0);
> 68:         Assert.assertEquals(dialog.getDialogPane().getHeight(), dialog2.getDialogPane().getHeight(), 0.0);
> 69:         hide();

The expected and actual values are backwards. When it fails, it's the first dialog that is wrong, so the expected values should come from dialog2.

tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java line 69:

> 68:         Assert.assertEquals(dialog.getDialogPane().getHeight(), dialog2.getDialogPane().getHeight(), 0.0);
> 69:         hide();
> 70:     }

When running the test on Linux without your fix, it gets the expected assertion failure, but then crashes. It looks like there is some interaction between the dialog still being open and the eventual call to `stage.hide()` and `Platform.exit`. I'll file a separate bug for this, since it is unrelated to your fix (I can make it happen with or without your fix).

I recommend putting the call to `hide()` in a try ... finally.

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



PR: https://git.openjdk.java.net/jfx/pull/63