RFR: 8359601: Fix window button states of an extended stage [v3]
Markus Mack
mmack at openjdk.org
Sat Jun 21 17:39:33 UTC 2025
On Tue, 17 Jun 2025 00:41:54 GMT, Michael Strauß <mstrauss at openjdk.org> 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<String> 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: https://git.openjdk.org/jfx/pull/1831#discussion_r2159441972
PR Review Comment: https://git.openjdk.org/jfx/pull/1831#discussion_r2159447743
More information about the openjfx-dev
mailing list