8359601: Fix window button states of an extended stage #1831

mstr2 · 2025-06-15T20:01:11Z

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

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)
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:

Unused code relating to window modality is removed.
The disabled states of HeaderButtonOverlay as well as HeaderButtonBehavior are changed to match the table above.
The stylesheets for GNOME and KDE are changed such that disabled buttons are hidden.
The stylesheet for Windows is changed such that a modal/non-resizable window looks like a utility window.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issues

JDK-8359601: Fix window button states of an extended stage (Bug - P4)
JDK-8359763: Close request handler is not called for an extended stage (Bug - P4)

Reviewers

Markus Mack (@drmarmac - Author)
Kevin Rushforth (@kevinrushforth - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1831/head:pull/1831
$ git checkout pull/1831

Update a local copy of the PR:
$ git checkout pull/1831
$ git pull https://git.openjdk.org/jfx.git pull/1831/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1831

View PR using the GUI difftool:
$ git pr show -t 1831

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1831.diff

Using Webrev

Link to Webrev Comment

mstr2 · 2025-06-15T20:01:28Z

/reviewers 2

bridgekeeper · 2025-06-15T20:01:56Z

👋 Welcome back mstrauss! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-06-15T20:02:46Z

@mstr2 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8359601: Fix window button states of an extended stage
8359763: Close request handler is not called for an extended stage

Reviewed-by: mmack, kcr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 27 new commits pushed to the master branch:

dbd43a3: 8363813: Missing null check in GlassScreen
7b59ebc: 8362873: Regression in BorderPane after JDK-8350149
da6965b: 8362079: Change JavaFX release version to 26
... and 24 more: https://git.openjdk.org/jfx/compare/fd30c94893156644c0d803b3e7fd8c9731d65fe6...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-06-15T20:03:07Z

@mstr2
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

mlbridge · 2025-06-15T20:06:11Z

Webrevs

credmond · 2025-06-16T23:09:26Z

Most of this seems to be working well!

But I have found a likely bug. Hopefully fairly self-explanatory from the code.

Minimise icon is normally disabled, but still enabled when EXTENDED is used & when dialog is resizable.

Just flip comment on dialog.initStyle(StageStyle.EXTENDED) to see difference.

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.ButtonType;
import javafx.scene.control.Dialog;
import javafx.scene.layout.StackPane;
import javafx.stage.Modality;
import javafx.stage.Stage;
import javafx.stage.StageStyle;

public class MinimiseIconEnabledBug extends Application {
    @Override
    public void start(Stage primaryStage) {
        Button button = new Button("Click");
        button.setOnAction(e -> {
            final Dialog<Object> dialog = new Dialog<>();
            dialog.initOwner(primaryStage);
            // Existing behaviour: minimise icon disabled when commented
            // Possible "bug": Minimise icon enabled when uncommented
            // dialog.initStyle(StageStyle.EXTENDED);
            dialog.initModality(Modality.NONE);
            dialog.setResizable(true); // This is important
            dialog.setTitle("My Dialog");
            dialog.setContentText("Lorem ipsum dolor sit amet, consectetur adipiscing...");
            dialog.getDialogPane().getButtonTypes().addAll(ButtonType.OK);
            dialog.show();
        });

        StackPane root = new StackPane(button);
        Scene scene = new Scene(root, 300, 200);

        primaryStage.setTitle("My First JavaFX App");
        primaryStage.setScene(scene);
        primaryStage.show();
    }

    public static void main(String[] args) {
        launch(args);
    }
}

credmond · 2025-06-17T00:37:22Z

Another suspected issue (not really related to this PR), close request handlers are not called when using EXTENDED. E.g.:

        stage.setOnCloseRequest(event -> {
            System.out.println("Never called...");;
        });

Same for dialogs...

mstr2 · 2025-06-17T00:38:31Z

Good catch, a non-modal owned window also can't be iconified. I've added code and tests for this scenario.

credmond · 2025-06-17T00:43:46Z

Another suspected issue (not really related to this PR), close request handlers are not called when using EXTENDED. E.g.:
        stage.setOnCloseRequest(event -> {
            System.out.println("Never called...");;
        });
Same for dialogs...

Sample:

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.ButtonType;
import javafx.scene.control.Dialog;
import javafx.scene.layout.StackPane;
import javafx.stage.Modality;
import javafx.stage.Stage;
import javafx.stage.StageStyle;

public class MinimiseIconEnabledBug extends Application {
    @Override
    public void start(Stage primaryStage) {
        Button button = new Button("Click");
        button.setOnAction(e -> {
            final Dialog<Object> dialog = new Dialog<>();
            dialog.initOwner(primaryStage);
            // dialog.initStyle(StageStyle.EXTENDED); // uncomment for bug
            dialog.initModality(Modality.NONE);
            dialog.setResizable(true); // This is important
            dialog.getDialogPane().getButtonTypes().addAll(ButtonType.OK);
            dialog.setOnCloseRequest(dialogEvent -> {
                System.out.println("Close the dialog pane. This is not called when EXTENDED is used.");
            });
            dialog.show();
        });

        StackPane root = new StackPane(button);
        Scene scene = new Scene(root, 300, 200);
        primaryStage.setScene(scene);
        primaryStage.show();
    }

    public static void main(String[] args) {
        launch(args);
    }
}

mstr2 · 2025-06-17T00:54:12Z

I've filed another ticket for that: JDK-8359763.

credmond · 2025-06-17T01:12:06Z

Good catch, a non-modal owned window also can't be iconified. I've added code and tests for this scenario.

Looks good, can't find anymore issues...

tsayao · 2025-06-19T00:21:58Z

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.

mstr2 · 2025-06-19T17:16:34Z

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.

drmarmac

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.

drmarmac · 2025-06-20T17:43:06Z

modules/javafx.graphics/src/main/java/com/sun/glass/ui/HeaderButtonOverlay.java

    private Node buttonAtMouseDown;

-    public HeaderButtonOverlay(ObservableValue<String> stylesheet, boolean utility, boolean rightToLeft) {
+    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?

drmarmac · 2025-06-20T17:45:11Z

modules/javafx.graphics/src/main/java/com/sun/javafx/scene/layout/HeaderButtonBehavior.java

        if (!node.disableProperty().isBound()) {
-            node.setDisable(resizable == Boolean.FALSE);
+            boolean utility = stage.getStyle() == StageStyle.UTILITY;
+            boolean modal = stage.getOwner() != null || stage.getModality() != Modality.NONE;


in case HeaderButtonOverlay.modal is renamed, this variable could be renamed in the same way.

drmarmac · 2025-06-20T17:50:25Z

modules/javafx.graphics/src/test/java/test/com/sun/glass/ui/HeaderButtonOverlayTest.java

+        }
+
+        final boolean resizable;
+        final boolean modal;


credmond · 2025-06-21T21:05:48Z

I've filed another ticket for that: 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?

mstr2 · 2025-06-21T21:56:42Z

I've filed another ticket for that: 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.

/issue add JDK-8359763

openjdk · 2025-06-21T21:57:22Z

@mstr2
Adding additional issue to issue list: 8359763: Close request handler is not called for an extended stage.

mstr2 · 2025-06-21T23:38:11Z

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.

drmarmac

Changes look good, all combinations that I've tested manually seem to work fine now. I also tested with gnome & KDE this time.

credmond · 2025-06-24T08:16:48Z

I have found another bug/issue, which is again related to wrapping a root node in a StackPane (something that is common in, for example, ControlsFx). Although the observed behaviour is probably strictly logically correct, I think this should be prevented (e.g, HeaderBar has/gets special meaning/treatment).

public class DisappearingHeaderBarNodesBug extends Application {

    @Override
    public void start(Stage primaryStage) {
        BorderPane root = new BorderPane();
        Button button = new Button("Click for \"bug()\"!");
        button.setOnAction(_ -> bug(root));
        root.setCenter(new VBox(new Label("Something above"), button, new Label("Something below")));
        root.setTop(getHeaderBar());

        primaryStage.initStyle(StageStyle.EXTENDED);
        primaryStage.setScene(new Scene(root, 300, 200));
        primaryStage.show();
    }

    // Wrapping the root node in a stackpane is common
    private void bug(Pane root) {
        StackPane stackPane = new StackPane();
        root.getScene().setRoot(stackPane);
        stackPane.getChildren().addFirst(root);
    }

    private HeaderBar getHeaderBar() {
        HeaderBar headerBar = new HeaderBar();
        headerBar.setCenter(new Label("!!! HeaderBar !!!"));
        headerBar.setLeading(new Label("L"));
        return headerBar;
    }

    public static void main(String[] args) {
        launch(args);
    }
}

As you can see, after you click the button, and resize, the HeaderBar can disappear completely:

bug.mp4

mstr2 · 2025-06-24T15:28:18Z

@credmond We should keep this PR focused, let's use the mailing list or JBS to discuss other issues.

credmond · 2025-07-07T23:45:08Z

FYI to any reviewer: I've been using this branch's EXTENDED/HeaderBar extensively now without any new issues...

mstr2 · 2025-07-22T18:28:47Z

I think this fix should make the 25 release if possible, as it would be unfortunate to preview the new feature with distracting bugs included. The fix is also quite well-tested at this point.

@kevinrushforth @andy-goryachev-oracle Do you have any review capacity to spare for this within the current RDP?

credmond · 2025-07-22T19:05:12Z

I think this fix should make the 25 release if possible, as it would be unfortunate to preview the new feature with distracting bugs included. The fix is also quite well-tested at this point.

@kevinrushforth @andy-goryachev-oracle Do you have any review capacity to spare for this within the current RDP?

Agreed. It wouldn't take long until others come across this and raise the same issue.

kevinrushforth · 2025-07-22T19:56:55Z

I think this fix should make the 25 release if possible, as it would be unfortunate to preview the new feature with distracting bugs included. The fix is also quite well-tested at this point.

@kevinrushforth @andy-goryachev-oracle Do you have any review capacity to spare for this within the current RDP?

Yes, I'll review it this week. It seems a good candidate for backporting to jfx25.

andy-goryachev-oracle · 2025-07-23T16:36:16Z

modules/javafx.graphics/src/main/java/com/sun/javafx/scene/layout/HeaderButtonBehavior.java

+        }
+
+        if (type == HeaderButtonType.MAXIMIZE) {
+            subscription = Subscription.combine(subscription,


here and on L66 the subscription pointer (set in L61) gets overwritten. does it mean some subscription(s) won't be cancelled in dispose()?

edit: please disregard, I see it's combining subscriptions.

kevinrushforth

The code looks good. I submitted a set of headful test jobs and will report when done. I want to do a little light manual testing, especially on macOS where there are a few changes in the non-extended case (although the code looks equivalent).

kevinrushforth · 2025-07-24T21:27:06Z

modules/javafx.graphics/src/main/native-glass/mac/GlassWindow+Java.m

 }

- (void)_setResizable
+- (void)_setResizable:(bool)resizable


Good. It definitely seems cleaner to pass in resizable as a parameter rather than having this method be a "toggle" resizable as it was previously.

kevinrushforth · 2025-07-24T21:28:43Z

modules/javafx.graphics/src/main/native-glass/mac/GlassWindow.m

-        {
-            [window performSelectorOnMainThread:@selector(_setResizable) withObject:nil waitUntilDone:YES];
-        }
+        [window _setResizable:jResizable];


I presume you removed the performSelectorOnMainThread because the calling method always calls this on the main thread?

Yes, this is a call that always happens on the FX thread as a result of calling javafx.stage.Window.show(). This is verified just a few lines earlier with GLASS_ASSERT_MAIN_JAVA_THREAD (that's always been there, I didn't add it).

Also, the calling Java code does a thread check. So we're good.

kevinrushforth · 2025-07-24T22:17:54Z

The headful CI tests passed on Linux and macOS (I will need to manually run it on Windows).

kevinrushforth · 2025-07-25T12:54:48Z

Headful tests passed on Windows.

mstr2 · 2025-07-25T13:00:41Z

/integrate

openjdk · 2025-07-25T13:01:09Z

Going to push as commit bc433da.
Since your change was applied there have been 27 commits pushed to the master branch:

dbd43a3: 8363813: Missing null check in GlassScreen
7b59ebc: 8362873: Regression in BorderPane after JDK-8350149
da6965b: 8362079: Change JavaFX release version to 26
... and 24 more: https://git.openjdk.org/jfx/compare/fd30c94893156644c0d803b3e7fd8c9731d65fe6...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-07-25T13:01:18Z

@mstr2 Pushed as commit bc433da.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

mstr2 · 2025-07-25T13:05:16Z

/backport jfx jfx25

openjdk · 2025-07-25T13:06:16Z

@mstr2 the backport was successfully created on the branch backport-mstr2-bc433da8-jfx25 in my personal fork of openjdk/jfx. To create a pull request with this backport targeting openjdk/jfx:jfx25, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit bc433da8 from the openjdk/jfx repository.

The commit being backported was authored by Michael Strauß on 25 Jul 2025 and was reviewed by Markus Mack and Kevin Rushforth.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jfx:

$ git fetch https://github.com/openjdk-bots/jfx.git backport-mstr2-bc433da8-jfx25:backport-mstr2-bc433da8-jfx25
$ git checkout backport-mstr2-bc433da8-jfx25
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jfx.git backport-mstr2-bc433da8-jfx25

kapilkumar9976 · 2025-09-20T07:03:01Z

Hi @kapilkumar9976, thanks for making a comment in an OpenJDK project!

All comments and discussions in the OpenJDK Community must be made available under the OpenJDK Terms of Use. If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please Use "Add GitHub user kapilkumar9976" for the summary.

If you are not an OpenJDK Author, Committer or Reviewer, simply check the box below to accept the OpenJDK Terms of Use for your comments.

I agree to the OpenJDK Terms of Use for all comments I make in a project in the OpenJDK GitHub organization.

Your comment will be automatically restored once you have accepted the OpenJDK Terms of Use.

mstr2 added 3 commits June 15, 2025 20:25

Remove unused window modality code

975d6ef

Fix disabled/hidden states of default window buttons

63fcaa6

Fix disabled states of custom window buttons

b3d7ae6

openjdk bot added the rfr Ready for review label Jun 15, 2025

Handle non-modal owned window correctly

06a0a5b

small refactor

ef05139

drmarmac reviewed Jun 21, 2025

View reviewed changes

rename modal to modalOrOwned

dcc21f7

fix window close request event

20eac39

fix resizable state of macOS window

cd550b4

drmarmac approved these changes Jun 22, 2025

View reviewed changes

andy-goryachev-oracle reviewed Jul 23, 2025

View reviewed changes

kevinrushforth self-requested a review July 24, 2025 19:21

kevinrushforth reviewed Jul 24, 2025

View reviewed changes

kevinrushforth approved these changes Jul 25, 2025

View reviewed changes

openjdk bot added the ready Ready to be integrated label Jul 25, 2025

openjdk bot added the integrated Pull request has been integrated label Jul 25, 2025

openjdk bot closed this Jul 25, 2025

openjdk bot removed ready Ready to be integrated rfr Ready for review labels Jul 25, 2025

mstr2 deleted the fixes/modal-window branch July 26, 2025 01:53

8359601: Fix window button states of an extended stage #1831

8359601: Fix window button states of an extended stage #1831

Uh oh!

Conversation

mstr2 commented Jun 15, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Windows 11

Ubuntu 24 / Fedora 41 (GNOME)

Kubuntu 24 (KDE)

Observations

Permitted window button operations

Fixes

Progress

Issues

Reviewers

Reviewing

Uh oh!

mstr2 commented Jun 15, 2025

Uh oh!

bridgekeeper bot commented Jun 15, 2025

Uh oh!

openjdk bot commented Jun 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jun 15, 2025

Uh oh!

mlbridge bot commented Jun 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

credmond commented Jun 16, 2025

Uh oh!

credmond commented Jun 17, 2025

Uh oh!

mstr2 commented Jun 17, 2025

Uh oh!

credmond commented Jun 17, 2025

Uh oh!

mstr2 commented Jun 17, 2025

Uh oh!

credmond commented Jun 17, 2025

Uh oh!

tsayao commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mstr2 commented Jun 19, 2025

Uh oh!

drmarmac left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drmarmac Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

drmarmac Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

drmarmac Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

credmond commented Jun 21, 2025

Uh oh!

mstr2 commented Jun 21, 2025

Uh oh!

openjdk bot commented Jun 21, 2025

Uh oh!

mstr2 commented Jun 21, 2025

Uh oh!

drmarmac left a comment

Choose a reason for hiding this comment

Uh oh!

credmond commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mstr2 commented Jun 24, 2025

Uh oh!

credmond commented Jul 7, 2025

Uh oh!

mstr2 commented Jul 22, 2025

Uh oh!

mstr2 commented Jun 15, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jun 15, 2025 •

edited

Loading

mlbridge bot commented Jun 15, 2025 •

edited

Loading

tsayao commented Jun 19, 2025 •

edited

Loading

drmarmac left a comment •

edited

Loading

credmond commented Jun 24, 2025 •

edited

Loading

andy-goryachev-oracle Jul 23, 2025 •

edited

Loading

kapilkumar9976 commented Sep 20, 2025 •

edited by bridgekeeper bot

Loading