8326497: Window.toFront() fails for iconified windows on Linux #17985

mkartashev · 2024-02-23T15:58:18Z

XToolkit implements toFront() essentially by sending the _NET_ACTIVE_WINDOW message to activate the window. This has no effect when the target window is in the iconified state. So the fix is to cancel that state prior to sending the message.

The test verifies that the fix works for maximized and undecorated frames as well out of abundance of caution; they do not represent a special case from the toFront() standpoint.

References

Progress

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

Issue

JDK-8326497: Window.toFront() fails for iconified windows on Linux (Bug - P4)

Reviewers

Tejesh R (@TejeshR13 - Committer)
Phil Race (@prrace - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 17985

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17985.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-02-23T15:58:41Z

👋 Welcome back mkartashev! 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 · 2024-02-23T15:59:58Z

@mkartashev The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2024-02-23T16:03:06Z

Webrevs

prrace · 2024-02-23T21:51:30Z

Looks reasonable to me. I'm a bit surprised this hasn't come up before.

On Mac I see that we already do this properly, I presume this is also the case on Windows, so the test will pass on all platforms ?

I think we should submit a test job ...

prrace · 2024-02-24T19:06:12Z

test/jdk/java/awt/Frame/Iconify/IconifiedToFront.java

+ * @summary Verifies that an iconified window is restored with Window.toFront()
+ * @requires os.family == "linux"
+ * @library /test/lib
+ * @run main IconifiedToFront


Well, the testing noticed for me that this fails because you don't specify
@key headless

Also the requires os.family == linux surely should not be needed
If this is the desired JDK behaviour it should do the same on all platforms.

I'm assuming you meant @key headful, right?

The test, unfortunately, fails both on macOS and Windows. I guess the same thing has be done there as well, but I don't have the hardware handy at the moment. Will get back to it later.

And made the test not dependent on swing.

mkartashev · 2024-03-05T09:24:23Z

@prrace The fix was extended to Windows as well. MacOS works as it is except that the test needed some tweaking to allow for slower iconify/de-iconify effects.

Please, have a look at the updated version.

TejeshR13 · 2024-03-05T09:31:25Z

test/jdk/java/awt/Frame/Iconify/IconifiedToFront.java

+    }
+
+    private static void pause() {
+        robot.delay(PAUSE_MS);


Adding robot.waitForIdle(); after delay would be better.

Agreed; done.

TejeshR13 · 2024-03-05T10:44:53Z

test/jdk/java/awt/Frame/Iconify/IconifiedToFront.java

+import java.awt.Toolkit;
+
+public class IconifiedToFront {
+    private static final int PAUSE_MS = 1500;


1.5 sec is minimum delay, can we reduced it ?

Not unless there's another idea on how to measure tests' success. 1.5 seconds is about the time my MacOS needs to complete an iconify/de-iconfiy process.

Did u try after adding waitForIdle ?

You were right @TejeshR13. waitForIdle helps and I reverted the time back to 500ms.

TejeshR13

Looks good to me.

mkartashev · 2024-03-06T11:40:39Z

@prrace What say you?

prrace · 2024-03-06T23:43:38Z

Let me submit a new test job with this (hopefully final?) version

prrace

Testing all good. Approved

openjdk · 2024-03-07T21:33:55Z

@mkartashev 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:

8326497: Window.toFront() fails for iconified windows on Linux

Reviewed-by: tr, prr

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 177 new commits pushed to the master branch:

2d4c757: 8327041: Incorrect lane size references in avx512 instructions.
761ed25: 8327138: Clean up status management in cdsConfig.hpp and CDS.java
53628f2: 8327492: Remove applet usage and update DisposeInActionEventTest.html
2627470: 8325567: jspawnhelper without args fails with segfault
a6dc4bc: 8326332: Unclosed inline tags cause misalignment in summary tables
33aa4b2: 8256314: JVM TI GetCurrentContendedMonitor is implemented incorrectly
87b40c6: 8327167: Clarify the handling of Leap year by Calendar
6efdaf8: 8326908: DecimalFormat::toPattern throws OutOfMemoryError when pattern is empty string
c4845f0: 8317431: Implement simpler Comparator when building certification paths
585a958: 8327675: jspawnhelper should be built on all unix platforms
... and 167 more: https://git.openjdk.org/jdk/compare/09a78b5da961f3575cf20c9b56bff86ddbd6545d...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@TejeshR13, @prrace) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

mkartashev · 2024-03-09T08:49:06Z

/integrate

openjdk · 2024-03-09T08:49:47Z

@mkartashev
Your change (at version da72dbc) is now ready to be sponsored by a Committer.

avu · 2024-03-12T08:14:36Z

/sponsor

openjdk · 2024-03-12T08:15:38Z

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

9f7aff4: 8327788: G1: Improve concurrent reference processing documentation
251347b: 8326139: C2 SuperWord: split packs (match use/def packs, implemented, mutual independence)
e21da4c: 8327486: java/util/Properties/PropertiesStoreTest.java fails "Text 'xxx' could not be parsed at index 20" after 8174269
4d6235e: 8327716: RISC-V: Change type of vector_length param of several assembler functions from int to uint
586396c: 8327705: Remove mention of "applet" from java.text package description
41450e9: 8314250: CDS dump error message: Invoker type parameter must start and end with Object: L3I_L
0a6e64e: 8327424: ProblemList serviceability/sa/TestJmapCore.java on all platforms with ZGC
c65d308: 8327812: JFR: Remove debug message in Function.Maximum
b9bc31f: 8327790: Improve javadoc for ResolvedJavaType.hasFinalizableSubclass
18de932: 8325991: Accelerate Poly1305 on x86_64 using AVX2 instructions
... and 186 more: https://git.openjdk.org/jdk/compare/09a78b5da961f3575cf20c9b56bff86ddbd6545d...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-03-12T08:15:44Z

@avu @mkartashev Pushed as commit 139681a.

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

8326497: Window.toFront() fails for iconified windows on Linux

429333a

openjdk bot added the rfr Pull request is ready for review label Feb 23, 2024

openjdk bot added the client client-libs-dev@openjdk.org label Feb 23, 2024

prrace reviewed Feb 24, 2024

View reviewed changes

mkartashev mentioned this pull request Feb 26, 2024

JBR-6737 Cannot switch projects from Window menu after minimizing them on Linux JetBrains/JetBrainsRuntime#317

Closed

mkartashev added 3 commits February 26, 2024 09:47

Extended the test to all platforms and made it headful

1b69d06

Adapted the test to the realities of macOS

35cef96

Fixed for Windows also

11060c7

And made the test not dependent on swing.

TejeshR13 reviewed Mar 5, 2024

View reviewed changes

Made the test to waitForIdle()

d7d134e

TejeshR13 reviewed Mar 5, 2024

View reviewed changes

Reduced the wait time in the test

da72dbc

TejeshR13 approved these changes Mar 6, 2024

View reviewed changes

prrace approved these changes Mar 7, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 7, 2024

openjdk bot added the sponsor Pull request is ready to be sponsored label Mar 9, 2024

openjdk bot added the integrated Pull request has been integrated label Mar 12, 2024

openjdk bot closed this Mar 12, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8326497: Window.toFront() fails for iconified windows on Linux #17985

8326497: Window.toFront() fails for iconified windows on Linux #17985

mkartashev commented Feb 23, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 23, 2024

openjdk bot commented Feb 23, 2024

mlbridge bot commented Feb 23, 2024 •

edited

Loading

prrace commented Feb 23, 2024

prrace Feb 24, 2024

mkartashev Feb 26, 2024

mkartashev commented Mar 5, 2024

TejeshR13 Mar 5, 2024

mkartashev Mar 5, 2024

TejeshR13 Mar 5, 2024

mkartashev Mar 5, 2024

TejeshR13 Mar 6, 2024

mkartashev Mar 6, 2024

TejeshR13 left a comment

mkartashev commented Mar 6, 2024

prrace commented Mar 6, 2024

prrace left a comment

openjdk bot commented Mar 7, 2024 •

edited

Loading

mkartashev commented Mar 9, 2024

openjdk bot commented Mar 9, 2024

avu commented Mar 12, 2024

openjdk bot commented Mar 12, 2024

openjdk bot commented Mar 12, 2024

8326497: Window.toFront() fails for iconified windows on Linux #17985

8326497: Window.toFront() fails for iconified windows on Linux #17985

Conversation

mkartashev commented Feb 23, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Feb 23, 2024

openjdk bot commented Feb 23, 2024

mlbridge bot commented Feb 23, 2024 • edited Loading

Webrevs

prrace commented Feb 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkartashev commented Mar 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TejeshR13 left a comment

Choose a reason for hiding this comment

mkartashev commented Mar 6, 2024

prrace commented Mar 6, 2024

prrace left a comment

Choose a reason for hiding this comment

openjdk bot commented Mar 7, 2024 • edited Loading

mkartashev commented Mar 9, 2024

openjdk bot commented Mar 9, 2024

avu commented Mar 12, 2024

openjdk bot commented Mar 12, 2024

openjdk bot commented Mar 12, 2024

mkartashev commented Feb 23, 2024 •

edited by openjdk bot

Loading

mlbridge bot commented Feb 23, 2024 •

edited

Loading

openjdk bot commented Mar 7, 2024 •

edited

Loading