8359899: Stage.isFocused() returns invalid value when Stage fails to receive focus

[lukostyra]

This PR fixes isFocused() returning invalid value when Stage fails to receive focus after calling Stage.show(). This problem is Windows-only.

In Windows the SetForegroundWindow() API lists a set of conditions to successfully grant focus to a Window. If any of the conditions are not met, the API will return FALSE. JavaFX did not respect that and instead assumed that receiving WM_ACTIVATE with our Window being activated is enough to assume the Window is in focus (which in some cases is not true).

I first tried reacting to WM_SETFOCUS and WM_KILLFOCUS but it seems those messages are not sent when the window is shown for the first time (instead WM_ACTIVATE is used).

To correct this behavior, I noticed the following path is the most reliable:

• Call ShowWindow() using SW_SHOWNA instead of SW_SHOW - that makes the window visible but does NOT activate it
• Call SetForegroundWindow() - that will attempt to give the Window focus and will also activate it if it is successful
  • If successful, Java notifyFocus callback will be called via WM_ACTIVATE handler
  • If it fails, we call the notifyFocus callback manually informing the upper layers the focus is lost. This establishes the correct state of Window.focused property.

With this change I observed that all tests pass as intended as long as two conditions are met (these are needed to satisfy SetForegroundWindow() restrictions):

• Gradle build is ran without the Gradle daemon
• The terminal running Gradle test is in foreground

If any of above two conditions is not met, some tests (including canary test from https://github.com/openjdk/jfx/pull/1804) now timeout/fail when checking whether Window.isFocused() is true.

Manually started JavaFX apps (ex. Ensemble) run as they used to and still receive focus upon Stage showing.

---

Progress

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

Issue

• JDK-8359899: Stage.isFocused() returns invalid value when Stage fails to receive focus (Bug - P3)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1849

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: Welcome back lkostyra! 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[bot]]

❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.

[mlbridge[bot]]

Webrevs

• 01: Full - Incremental (00c2d60e)
• 00: Full (f8a575e4)

[bridgekeeper[bot]]

@lukostyra This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[lukostyra]

/keepalive

[openjdk[bot]]

@lukostyra The pull request is being re-evaluated and the inactivity timeout has been reset.

[beldenfox]

I’ve been testing this PR by launching the manual KeyboardTest app.

java @build/run.args tests/manual/events/KeyboardTest.java

The call to SetForegroundWindow fails but not before activating the window. Since it’s not the foreground window this PR correctly sets the JavaFX focused property back to false. But when I later click on the title bar to bring the window to the foreground the focused property is not updated.

I’m not sure how we’re expected to detect that our HWND has come to the foreground. There’s no specific message sent when this happens and since the OS thinks the window is already active and focused we don’t get WM_ACTIVATE or WM_SETFOCUS. There’s some messages we could use heuristically (like WM_NCACTIVATE) but I couldn’t find anything more clear cut. I'll keep looking but it doesn't look like Windows makes this easy.

[lukostyra]

@beldenfox thanks for checking. I remember doing a fair share of looking for a reliable answer to this and couldn't find anything specific, other that SetForegroundWindow being able to fail and having to "handle it". I'll get back to testing this starting next week as well, maybe we'll manage to track down some more reasonable solution.

[bridgekeeper[bot]]

@lukostyra This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[lukostyra]

/touch

I didn't get a chance to look into other solutions yet, but I will be investigating that soon.

[openjdk[bot]]

@lukostyra The pull request is being re-evaluated and the inactivity timeout has been reset.

[lukostyra]

The call to SetForegroundWindow fails but not before activating the window. Since it’s not the foreground window this PR correctly sets the JavaFX focused property back to false. But when I later click on the title bar to bring the window to the foreground the focused property is not updated.

I played around with this for a bit and I cannot reproduce this behavior. When I start the app it gets brought to foreground and stage.isFocused() returns true as it should for me.

Could it be the difference between Windows versions maybe? I'm running Win 11 24H2 at this point in time.

[kevinrushforth]

Reviewers: @beldenfox @kevinrushforth

/reviewers 2

[openjdk[bot]]

@kevinrushforth 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).

[beldenfox]

I played around with this for a bit and I cannot reproduce this behavior. When I start the app it gets brought to foreground and stage.isFocused() returns true as it should for me.

Could it be the difference between Windows versions maybe? I'm running Win 11 24H2 at this point in time.

I'm also running an up-to-date version of Win 11. I can reproduce the ::SetForegroundWindow failure when launching from a MinGW terminal window but not from a Cygwin terminal.

[bridgekeeper[bot]]

@lukostyra This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!