Re: RFR: 8251241: macOS: iconify property doesn't change after minimize when resizable is false [v2]
On Thu, 13 Aug 2020 18:11:25 GMT, Kevin Rushforth wrote: >> Thank you for the very quick feedback! >> In the application, the fix was developed for, it also seems to fix all >> issues related to iconified. > > You may have seen my comment in JBS: This PR also fixes > [JDK-8249202](https://bugs.openjdk.java.net/browse/JDK-8249202), which I > discovered while testing > [JDK-8248490](https://bugs.openjdk.java.net/browse/JDK-8248490). This means > that I already have an automated test that > can be modified to test this. I just need to add a couple things to it (an > assertion check for the iconified property, > a check to ensure that we can deiconify the stage, and then add a mode to run > it with a non-resizable stage). Once I > verify it, I can send you the patch for the test, if that's OK with you? Here is [the patch](https://github.com/openjdk/jfx/files/5071008/icontest.patch.txt) for [IconifyTest.java](https://github.com/openjdk/jfx/blob/1f42cebed04d88998d1a66ce99ba3c51995a18a8/tests/system/src/test/java/test/robot/javafx/stage/IconifyTest.java) : [icontest.patch.txt](https://github.com/openjdk/jfx/files/5071008/icontest.patch.txt) I verified that it catches the bugs, in that the modified test fails without the fix and passes with the fix. Can you apply the patch to your repo and push a new commit with the updated test? - PR: https://git.openjdk.java.net/jfx/pull/280
Re: RFR: 8251241: macOS: iconify property doesn't change after minimize when resizable is false [v2]
On Thu, 13 Aug 2020 17:09:43 GMT, Florian Kirmaier wrote: >> This looks good based on my initial testing, including a modified version of >> the test program that doesn't temporarily >> set resizable to true. Nice. >> I want to do some additional testing. >> >> I left one minor comment on the code formatting inline. > > Thank you for the very quick feedback! > In the application, the fix was developed for, it also seems to fix all > issues related to iconified. You may have seen my comment in JBS: This PR also fixes [JDK-8249202](https://bugs.openjdk.java.net/browse/JDK-8249202), which I discovered while testing [JDK-8248490](https://bugs.openjdk.java.net/browse/JDK-8248490). This means that I already have an automated test that can be modified to test this. I just need to add a couple things to it (an assertion check for the iconified property, a check to ensure that we can deiconify the stage, and then add a mode to run it with a non-resizable stage). Once I verify it, I can send you the patch for the test, if that's OK with you? - PR: https://git.openjdk.java.net/jfx/pull/280
Re: RFR: 8251241: macOS: iconify property doesn't change after minimize when resizable is false [v2]
On Thu, 13 Aug 2020 16:35:51 GMT, Kevin Rushforth wrote:
>> Florian Kirmaier has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> JDK-8251241
>> Added missing spaces to match the coding style.
>
> modules/javafx.graphics/src/main/native-glass/mac/GlassWindow+Java.m line 100:
>
>> 99: type = com_sun_glass_events_WindowEvent_MINIMIZE;
>> 100: } else if([self->nsWindow isZoomed]) {
>> 101: type = com_sun_glass_events_WindowEvent_MAXIMIZE;
>
> Minor: can you add a space between `if` and `(` to match our coding style?
Done!
-
PR: https://git.openjdk.java.net/jfx/pull/280
Re: RFR: 8251241: macOS: iconify property doesn't change after minimize when resizable is false [v2]
On Thu, 13 Aug 2020 16:37:44 GMT, Kevin Rushforth wrote: >> Florian Kirmaier has updated the pull request incrementally with one >> additional commit since the last revision: >> >> JDK-8251241 >> Added missing spaces to match the coding style. > > This looks good based on my initial testing, including a modified version of > the test program that doesn't temporarily > set resizable to true. Nice. > I want to do some additional testing. > > I left one minor comment on the code formatting inline. Thank you for the very quick feedback! In the application, the fix was developed for, it also seems to fix all issues related to iconified. - PR: https://git.openjdk.java.net/jfx/pull/280
Re: RFR: 8251241: macOS: iconify property doesn't change after minimize when resizable is false [v2]
> ticket: https://bugs.openjdk.java.net/browse/JDK-8251241 > > This small change fixing the minimize for mac. > Changes are property registered. > Calling setIconize(true) no longer resets to false. Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision: JDK-8251241 Added missing spaces to match the coding style. - Changes: - all: https://git.openjdk.java.net/jfx/pull/280/files - new: https://git.openjdk.java.net/jfx/pull/280/files/f89e43f2..1f42cebe Webrevs: - full: https://webrevs.openjdk.java.net/jfx/280/webrev.01 - incr: https://webrevs.openjdk.java.net/jfx/280/webrev.00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/280.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/280/head:pull/280 PR: https://git.openjdk.java.net/jfx/pull/280
