Re: RFR: 8359601: Fix window button states of an extended stage [v3]
On Sat, 21 Jun 2025 17:37:02 GMT, Markus Mack wrote: > I also checked the behavior in macOS Sequoia 15.5. There, a window where > `setResizable(false)` is called before showing the stage, unexpectedly has a > working maximize button even without `StageStyle.EXTENDED`. Looks like this > was not the case prior to the integration of #1605, so this behavior was > probably introduced there and may need a fix in this PR or elsewhere. The implementation relied on the `_setResizable` JNI method being called in a very particular way that has since changed (internally, it would call another method with the same name, but the implementation would actually _toggle_ the resizable state, not _set_ it to a given value). I've fixed this problem. - PR Comment: https://git.openjdk.org/jfx/pull/1831#issuecomment-2993820442
Re: RFR: 8359601: Fix window button states of an extended stage [v3]
On Sat, 21 Jun 2025 21:05:48 GMT, Cormac Redmond wrote: > > I've filed another ticket for that: > > [JDK-8359763](https://bugs.openjdk.org/browse/JDK-8359763). > > Just curious, why a separate bug for this instead of fixing in this PR and > avoiding the headaches/conversations of more PRs -- is it more complicated > that it sounds? Shorter PRs tend to get more timely reviews. That being said, since it is closely related, I've added the fix to this PR. - PR Comment: https://git.openjdk.org/jfx/pull/1831#issuecomment-2993788403
Re: RFR: 8359601: Fix window button states of an extended stage [v3]
On Tue, 17 Jun 2025 00:54:12 GMT, Michael Strauß wrote: > I've filed another ticket for that: > [JDK-8359763](https://bugs.openjdk.org/browse/JDK-8359763). Just curious, why a separate bug for this instead of fixing in this PR and avoiding the headaches/conversations of more PRs -- is it more complicated that it sounds? - PR Comment: https://git.openjdk.org/jfx/pull/1831#issuecomment-2993771128
Re: RFR: 8359601: Fix window button states of an extended stage [v3]
On Tue, 17 Jun 2025 00:41:54 GMT, Michael Strauß wrote:
>> The window button states (disabled/hidden) of extended stages with a
>> `HeaderButtonOverlay` or custom header buttons are inconsistent with what we
>> would expect from the OS (Windows and Linux). To figure out what we would
>> expect, I started with gathering some data. The following table shows the
>> button states of system-decorated windows on various platforms:
>>
>> Windows 11
>>
>> | Window attributes | Iconify | Maximize | Close |
>> |---|---|---|---|
>> | resizable, not modal | visible | visible | visible |
>> | not resizable, not modal | visible | visible, disabled | visible |
>> | resizable, modal | visible, disabled | visible | visible |
>> | not resizable, modal | hidden | hidden | visible, utility-style |
>>
>> Ubuntu 24 / Fedora 41 (GNOME)
>>
>> | Window attributes | Iconify | Maximize | Close |
>> |---|---|---|---|
>> | resizable, not modal | visible | visible | visible |
>> | not resizable, not modal | visible | hidden | visible |
>> | resizable, modal | visible, _not working_ | visible, _not working_ |
>> visible |
>> | not resizable, modal | visible, _not working_ | hidden | visible |
>>
>> Kubuntu 24 (KDE)
>>
>> | Window attributes | Iconify | Maximize | Close |
>> |---|---|---|---|
>> | resizable, not modal | visible | visible | visible |
>> | not resizable, not modal | visible | hidden | visible |
>> | resizable, modal | visible, _not working_ | visible | visible |
>> | not resizable, modal | visible, _not working_ | hidden | visible |
>>
>> ## Observations
>>
>> 1. On Windows, buttons are generally disabled when their operation is not
>> possible with the given window attributes.
>>* Exception: modal/non-resizable windows look like utility windows
>> (iconify and maximize are hidden)
>> 2. On GNOME and KDE, buttons are generally hidden when their operation is
>> not possible.
>>* Exception: iconify and maximize on modal windows is not hidden, but
>> seems to simply not do anything (bug?)
>>
>> ## Permitted window button operations
>>
>> Given the gathered observations and some simple logic, this is the table of
>> operations that are permitted for all combinations of modal and resizable
>> window attributes:
>>
>> | Window attributes | Iconify | Maximize | Close |
>> |---|---|---|---|
>> | resizable, not modal | yes | yes | yes |
>> | not resizable, not modal | yes | no | yes |
>> | resizable, modal | no | yes | yes |
>> | not resizable, modal | no | no | yes |
>>
>> ## Fixes
>>
>> This PR includes the following changes:
>> 1. Unused code relating to window modality is removed...
>
> Michael Strauß has updated the pull request incrementally with one additional
> commit since the last revision:
>
> small refactor
The updated behavior looks good on windows 11, I tested various combinations of
modal/owned/resizable/StageStyle settings, including modifying the resizeable
bit when a stage is already showing.
I also checked the behavior in macOS Sequoia 15.5. There, a window where
`setResizable(false)` is called before showing the stage, unexpectedly has a
working maximize button even without `StageStyle.EXTENDED`.
Looks like this was not the case prior to the integration of #1605, so this
behavior was probably introduced there and may need a fix in this PR or
elsewhere.
I don't see any code using the java and native enterModal/exitModal methods
which were removed, so this looks like a safe change.
Finally, I left some comments regarding potentially confusing definitions of
`modal`, otherwise the code looks good.
modules/javafx.graphics/src/main/java/com/sun/glass/ui/HeaderButtonOverlay.java
line 272:
> 270: private Node buttonAtMouseDown;
> 271:
> 272: public HeaderButtonOverlay(ObservableValue stylesheet,
> boolean modal,
it's a bit confusing to have `isModal` in `Window` and `getModality` in `Stage`
on one hand, and a `modal` field / parameter here, which is set to `isModal()
|| getOwner() != null` by callers. Maybe clarify by renaming to something like
`effecitvelyModal` or `modalOrOwned`?
modules/javafx.graphics/src/main/java/com/sun/javafx/scene/layout/HeaderButtonBehavior.java
line 158:
> 156: if (!node.disableProperty().isBound()) {
> 157: boolean utility = stage.getStyle() == StageStyle.UTILITY;
> 158: boolean modal = stage.getOwner() != null ||
> stage.getModality() != Modality.NONE;
in case `HeaderButtonOverlay.modal` is renamed, this variable could be renamed
in the same way.
modules/javafx.graphics/src/test/java/test/com/sun/glass/ui/HeaderButtonOverlayTest.java
line 348:
> 346:
> 347: final boolean resizable;
> 348: final boolean modal;
same here
-
PR Review: https://git.openjdk.org/jfx/pull/1831#pullrequestreview-2946917903
PR Review Comment: https://git.openjdk.org/jfx/pull/1831#discussion_r2159439631
PR Review Comment:
Re: RFR: 8359601: Fix window button states of an extended stage [v3]
On Tue, 17 Jun 2025 00:41:54 GMT, Michael Strauß wrote: >> The window button states (disabled/hidden) of extended stages with a >> `HeaderButtonOverlay` or custom header buttons are inconsistent with what we >> would expect from the OS (Windows and Linux). To figure out what we would >> expect, I started with gathering some data. The following table shows the >> button states of system-decorated windows on various platforms: >> >> Windows 11 >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | visible | visible | visible | >> | not resizable, not modal | visible | visible, disabled | visible | >> | resizable, modal | visible, disabled | visible | visible | >> | not resizable, modal | hidden | hidden | visible, utility-style | >> >> Ubuntu 24 / Fedora 41 (GNOME) >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | visible | visible | visible | >> | not resizable, not modal | visible | hidden | visible | >> | resizable, modal | visible, _not working_ | visible, _not working_ | >> visible | >> | not resizable, modal | visible, _not working_ | hidden | visible | >> >> Kubuntu 24 (KDE) >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | visible | visible | visible | >> | not resizable, not modal | visible | hidden | visible | >> | resizable, modal | visible, _not working_ | visible | visible | >> | not resizable, modal | visible, _not working_ | hidden | visible | >> >> ## Observations >> >> 1. On Windows, buttons are generally disabled when their operation is not >> possible with the given window attributes. >>* Exception: modal/non-resizable windows look like utility windows >> (iconify and maximize are hidden) >> 2. On GNOME and KDE, buttons are generally hidden when their operation is >> not possible. >>* Exception: iconify and maximize on modal windows is not hidden, but >> seems to simply not do anything (bug?) >> >> ## Permitted window button operations >> >> Given the gathered observations and some simple logic, this is the table of >> operations that are permitted for all combinations of modal and resizable >> window attributes: >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | yes | yes | yes | >> | not resizable, not modal | yes | no | yes | >> | resizable, modal | no | yes | yes | >> | not resizable, modal | no | no | yes | >> >> ## Fixes >> >> This PR includes the following changes: >> 1. Unused code relating to window modality is removed... > > Michael Strauß has updated the pull request incrementally with one additional > commit since the last revision: > > small refactor > JavaFX modal windows are not native modals, as evidenced by the code removal > on this PR (it's not used) . On GNOME, native modal windows automatically > remove the minimize and maximize buttons. > > I did an experiment and found that it's possible to get native modal windows > (at least with GTK Glass) by passing the modality flag from `WindowStage` to > `createWindow`. I haven’t done extensive testing, but it seems to work - > likely because the modality can’t be changed afterward. That's a good idea for an enhancement. - PR Comment: https://git.openjdk.org/jfx/pull/1831#issuecomment-2988750400
Re: RFR: 8359601: Fix window button states of an extended stage [v3]
On Tue, 17 Jun 2025 00:41:54 GMT, Michael Strauß wrote: >> The window button states (disabled/hidden) of extended stages with a >> `HeaderButtonOverlay` or custom header buttons are inconsistent with what we >> would expect from the OS (Windows and Linux). To figure out what we would >> expect, I started with gathering some data. The following table shows the >> button states of system-decorated windows on various platforms: >> >> Windows 11 >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | visible | visible | visible | >> | not resizable, not modal | visible | visible, disabled | visible | >> | resizable, modal | visible, disabled | visible | visible | >> | not resizable, modal | hidden | hidden | visible, utility-style | >> >> Ubuntu 24 / Fedora 41 (GNOME) >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | visible | visible | visible | >> | not resizable, not modal | visible | hidden | visible | >> | resizable, modal | visible, _not working_ | visible, _not working_ | >> visible | >> | not resizable, modal | visible, _not working_ | hidden | visible | >> >> Kubuntu 24 (KDE) >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | visible | visible | visible | >> | not resizable, not modal | visible | hidden | visible | >> | resizable, modal | visible, _not working_ | visible | visible | >> | not resizable, modal | visible, _not working_ | hidden | visible | >> >> ## Observations >> >> 1. On Windows, buttons are generally disabled when their operation is not >> possible with the given window attributes. >>* Exception: modal/non-resizable windows look like utility windows >> (iconify and maximize are hidden) >> 2. On GNOME and KDE, buttons are generally hidden when their operation is >> not possible. >>* Exception: iconify and maximize on modal windows is not hidden, but >> seems to simply not do anything (bug?) >> >> ## Permitted window button operations >> >> Given the gathered observations and some simple logic, this is the table of >> operations that are permitted for all combinations of modal and resizable >> window attributes: >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | yes | yes | yes | >> | not resizable, not modal | yes | no | yes | >> | resizable, modal | no | yes | yes | >> | not resizable, modal | no | no | yes | >> >> ## Fixes >> >> This PR includes the following changes: >> 1. Unused code relating to window modality is removed... > > Michael Strauß has updated the pull request incrementally with one additional > commit since the last revision: > > small refactor JavaFX modal windows are not native modals, as evidenced by the code removal on this PR (it's not used) . On GNOME, native modal windows automatically remove the minimize and maximize buttons.  I did an experiment and found that it's possible to get native modal windows (at least with GTK Glass) by passing the modality flag from `WindowStage` to `createWindow`. I haven’t done extensive testing, but it seems to work - likely because the modality can’t be changed afterward. - PR Comment: https://git.openjdk.org/jfx/pull/1831#issuecomment-2986109841
Re: RFR: 8359601: Fix window button states of an extended stage [v3]
On Tue, 17 Jun 2025 00:41:54 GMT, Michael Strauß wrote: >> The window button states (disabled/hidden) of extended stages with a >> `HeaderButtonOverlay` or custom header buttons are inconsistent with what we >> would expect from the OS (Windows and Linux). To figure out what we would >> expect, I started with gathering some data. The following table shows the >> button states of system-decorated windows on various platforms: >> >> Windows 11 >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | visible | visible | visible | >> | not resizable, not modal | visible | visible, disabled | visible | >> | resizable, modal | visible, disabled | visible | visible | >> | not resizable, modal | hidden | hidden | visible, utility-style | >> >> Ubuntu 24 / Fedora 41 (GNOME) >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | visible | visible | visible | >> | not resizable, not modal | visible | hidden | visible | >> | resizable, modal | visible, _not working_ | visible, _not working_ | >> visible | >> | not resizable, modal | visible, _not working_ | hidden | visible | >> >> Kubuntu 24 (KDE) >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | visible | visible | visible | >> | not resizable, not modal | visible | hidden | visible | >> | resizable, modal | visible, _not working_ | visible | visible | >> | not resizable, modal | visible, _not working_ | hidden | visible | >> >> ## Observations >> >> 1. On Windows, buttons are generally disabled when their operation is not >> possible with the given window attributes. >>* Exception: modal/non-resizable windows look like utility windows >> (iconify and maximize are hidden) >> 2. On GNOME and KDE, buttons are generally hidden when their operation is >> not possible. >>* Exception: iconify and maximize on modal windows is not hidden, but >> seems to simply not do anything (bug?) >> >> ## Permitted window button operations >> >> Given the gathered observations and some simple logic, this is the table of >> operations that are permitted for all combinations of modal and resizable >> window attributes: >> >> | Window attributes | Iconify | Maximize | Close | >> |---|---|---|---| >> | resizable, not modal | yes | yes | yes | >> | not resizable, not modal | yes | no | yes | >> | resizable, modal | no | yes | yes | >> | not resizable, modal | no | no | yes | >> >> ## Fixes >> >> This PR includes the following changes: >> 1. Unused code relating to window modality is removed... > > Michael Strauß has updated the pull request incrementally with one additional > commit since the last revision: > > small refactor I've filed another ticket for that: [JDK-8359763](https://bugs.openjdk.org/browse/JDK-8359763). - PR Comment: https://git.openjdk.org/jfx/pull/1831#issuecomment-2978589007
Re: RFR: 8359601: Fix window button states of an extended stage [v3]
> The window button states (disabled/hidden) of extended stages with a > `HeaderButtonOverlay` or custom header buttons are inconsistent with what we > would expect from the OS (Windows and Linux). To figure out what we would > expect, I started with gathering some data. The following table shows the > button states of system-decorated windows on various platforms: > > Windows 11 > > | Window attributes | Iconify | Maximize | Close | > |---|---|---|---| > | resizable, not modal | visible | visible | visible | > | not resizable, not modal | visible | visible, disabled | visible | > | resizable, modal | visible, disabled | visible | visible | > | not resizable, modal | hidden | hidden | visible, utility-style | > > Ubuntu 24 / Fedora 41 (GNOME) > > | Window attributes | Iconify | Maximize | Close | > |---|---|---|---| > | resizable, not modal | visible | visible | visible | > | not resizable, not modal | visible | hidden | visible | > | resizable, modal | visible, _not working_ | visible, _not working_ | > visible | > | not resizable, modal | visible, _not working_ | hidden | visible | > > Kubuntu 24 (KDE) > > | Window attributes | Iconify | Maximize | Close | > |---|---|---|---| > | resizable, not modal | visible | visible | visible | > | not resizable, not modal | visible | hidden | visible | > | resizable, modal | visible, _not working_ | visible | visible | > | not resizable, modal | visible, _not working_ | hidden | visible | > > ## Observations > > 1. On Windows, buttons are generally disabled when their operation is not > possible with the given window attributes. >* Exception: modal/non-resizable windows look like utility windows > (iconify and maximize are hidden) > 2. On GNOME and KDE, buttons are generally hidden when their operation is not > possible. >* Exception: iconify and maximize on modal windows is not hidden, but > seems to simply not do anything (bug?) > > ## Permitted window button operations > > Given the gathered observations and some simple logic, this is the table of > operations that are permitted for all combinations of modal and resizable > window attributes: > > | Window attributes | Iconify | Maximize | Close | > |---|---|---|---| > | resizable, not modal | yes | yes | yes | > | not resizable, not modal | yes | no | yes | > | resizable, modal | no | yes | yes | > | not resizable, modal | no | no | yes | > > ## Fixes > > This PR includes the following changes: > 1. Unused code relating to window modality is removed. > 2. The disabled states of `HeaderButtonOverlay` as well as > `HeaderButtonBehavior` are changed to mat... Michael Strauß has updated the pull request incrementally with one additional commit since the last revision: small refactor - Changes: - all: https://git.openjdk.org/jfx/pull/1831/files - new: https://git.openjdk.org/jfx/pull/1831/files/06a0a5bb..ef05139e Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1831&range=02 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1831&range=01-02 Stats: 6 lines in 1 file changed: 1 ins; 4 del; 1 mod Patch: https://git.openjdk.org/jfx/pull/1831.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1831/head:pull/1831 PR: https://git.openjdk.org/jfx/pull/1831
