Re: [Integrated] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
Changeset: 1952606a Author:Thiago Milczarek Sayao Committer: Kevin Rushforth Date: 2020-01-02 17:01:13 + URL: https://git.openjdk.java.net/jfx/commit/1952606a 8232811: Dialog's preferred size no longer accommodates multi-line strings Reviewed-by: kcr, arapte ! modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp + tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java
Re: [Rev 03] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
On Fri, 20 Dec 2019 13:53:43 GMT, Kevin Rushforth wrote: >> The pull request has been updated with 2 additional commits. > > Looks good. > > I can sponsor this fix. @tsayao this is ready for you to integrate. - PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 03] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
On Fri, 20 Dec 2019 13:54:14 GMT, Thiago Milczarek Sayao 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 2 additional commits. Looks good. I can sponsor this fix. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 03] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
On Fri, 20 Dec 2019 13:13:55 GMT, Thiago Milczarek Sayao 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 2 additional commits. Looks good to me. Test passes on all three platforms. - Marked as reviewed by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
On Tue, 17 Dec 2019 21:37:34 GMT, Kevin Rushforth wrote:
>> The pull request has been updated with 1 additional commit.
>
> 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.
Added 2 pixels tolerance.
-
PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
On Thu, 19 Dec 2019 14:13:47 GMT, Ambarish Rapte wrote:
>> The pull request has been updated with 1 additional commit.
>
> tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java
> line 64:
>
>> 63: public void dialogWithOwnerSizingTest() throws Exception {
>> 64: Thread.sleep(500);
>> 65: clickButton();
>
> This `sleep()` can be removed.
Removed it, but the test failed.
-
PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 03] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
> 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 2 additional commits. - Added commits: - 1d40aa9f: Merge remote-tracking branch 'origin/fix_jdk_8232811' into fix_jdk_8232811 - f97833db: Did requested changes. Changes: - all: https://git.openjdk.java.net/jfx/pull/63/files - new: https://git.openjdk.java.net/jfx/pull/63/files/7b723c3b..1d40aa9f Webrevs: - full: https://webrevs.openjdk.java.net/jfx/63/webrev.03 - incr: https://webrevs.openjdk.java.net/jfx/63/webrev.02-03 Stats: 19 lines in 2 files changed: 8 ins; 9 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/63.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/63/head:pull/63 PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
On Tue, 17 Dec 2019 23:01:51 GMT, Kevin Rushforth wrote: >> The pull request has been updated with 1 additional commit. > > tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java > line 124: > >> 123: >> 124: stage.initStyle(StageStyle.UNDECORATED); >> 125: stage.setOnShown(e -> >> Platform.runLater(startupLatch::countDown)); > > Since this is a Robot test that relies on the Stage being in front of other > windows, can you add the following? > > stage.setAlwaysOnTop(true); ok - PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
On Thu, 19 Dec 2019 14:29:39 GMT, Ambarish Rapte wrote: >> The pull request has been updated with 1 additional commit. > > modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1062: > >> 1061: geometry.final_height.type = BOUNDSTYPE_CONTENT; >> 1062: } >> 1063: > > So now `geometry.final_width.value` and `geometry.final_height.value` will be > set after the `Dialog `is displayed on screen. > Does this mean that, the `Dialog` will be resized after it is displayed ? I > did not observe any size change visually. > > An observation unrelated to this change: `Dialog` changes it's position after > being displayed. It can not be observed always, but is very frequent. Should > be easy to observe it on a slow machine. > On the same machine, I did not observe any size change though. No, it will be set on set_bounds(). This will just update de final value after show() / map. - PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
On Thu, 19 Dec 2019 14:32:32 GMT, Thiago Milczarek Sayao
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 itself looks good. suggested some changes in test and a query.
Please take a look.
tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java
line 34:
> 33: import javafx.scene.control.Dialog;
> 34: import javafx.scene.control.DialogEvent;
> 35: import javafx.scene.robot.Robot;
DialogEvent is not used in program, please remove the import.
tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java
line 50:
> 49:
> 50: //see JDK8193502
> 51: public class DialogWithOwnerSizingTest {
Please change to JDK-8193502
tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java
line 64:
> 63: public void dialogWithOwnerSizingTest() throws Exception {
> 64: Thread.sleep(500);
> 65: clickButton();
This `sleep()` can be removed.
tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java
line 87:
> 86: });
> 87: Thread.sleep(600);
> 88: waitForLatch(dialogHideLatch, 10, "Failed to hide Dialog");
This `sleep()` can be removed too. anyway `dailogHideLatch` will synchronize.
tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java
line 77:
> 76:
> 77: Thread.sleep(400);
> 78: waitForLatch(dialogShownLatch, 10, "Failed to show Dialog");
This sleep() can be removed too. anyway `dailogShowLatch `will synchronize.
modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1062:
> 1061: geometry.final_height.type = BOUNDSTYPE_CONTENT;
> 1062: }
> 1063:
So now `geometry.final_width.value` and `geometry.final_height.value` will be
set after the `Dialog `is displayed on screen.
Does this mean that, the `Dialog` will be resized after it is displayed ? I did
not observe any size change visually.
An observation unrelated to this change: `Dialog` changes it's position after
being displayed. It can not be observed always, but is very frequent. Should be
easy to observe it on a slow machine.
On the same machine, I did not observe any size change though.
-
Changes requested by arapte (Reviewer).
PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
On Tue, 17 Dec 2019 21:43:19 GMT, Kevin Rushforth wrote: >> 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. @arapte - can you be the second reviewer on this? - PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
On Tue, 17 Dec 2019 23:02:02 GMT, Thiago Milczarek Sayao 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. tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java line 124: > 123: > 124: stage.initStyle(StageStyle.UNDECORATED); > 125: stage.setOnShown(e -> > Platform.runLater(startupLatch::countDown)); Since this is a Robot test that relies on the Stage being in front of other windows, can you add the following? stage.setAlwaysOnTop(true); - PR: https://git.openjdk.java.net/jfx/pull/63
Re: RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
On Mon, 9 Dec 2019 18:27:25 GMT, Thiago Milczarek Sayao 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 This will need two reviewers. - PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 02] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
> 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. - Added commits: - 7b723c3b: Revert unecessary change Changes: - all: https://git.openjdk.java.net/jfx/pull/63/files - new: https://git.openjdk.java.net/jfx/pull/63/files/7a8377e7..7b723c3b Webrevs: - full: https://webrevs.openjdk.java.net/jfx/63/webrev.02 - incr: https://webrevs.openjdk.java.net/jfx/63/webrev.01-02 Stats: 14 lines in 1 file changed: 8 ins; 6 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/63.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/63/head:pull/63 PR: https://git.openjdk.java.net/jfx/pull/63
Re: [Rev 01] RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
> 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. - Added commits: - 7a8377e7: Asthetics Changes: - all: https://git.openjdk.java.net/jfx/pull/63/files - new: https://git.openjdk.java.net/jfx/pull/63/files/338bedbe..7a8377e7 Webrevs: - full: https://webrevs.openjdk.java.net/jfx/63/webrev.01 - incr: https://webrevs.openjdk.java.net/jfx/63/webrev.00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/63.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/63/head:pull/63 PR: https://git.openjdk.java.net/jfx/pull/63
RFR: 8232811: Dialog's preferred size no longer accommodates multi-line strings
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 - Commits: - 338bedbe: Fix mistake - a02894d6: Fix JDK-8232811 - 7a23cdbd: Merge pull request #2 from openjdk/master - fc36a292: Merge pull request #1 from openjdk/master Changes: https://git.openjdk.java.net/jfx/pull/63/files Webrev: https://webrevs.openjdk.java.net/jfx/63/webrev.00 Issue: https://bugs.openjdk.java.net/browse/JDK-8232811 Stats: 188 lines in 2 files changed: 176 ins; 11 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/63.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/63/head:pull/63 PR: https://git.openjdk.java.net/jfx/pull/63
