8251240: Menus inaccessible on Linux with i3 wm #1173

tsayao · 2023-07-09T17:43:05Z

The bug happens because gdk_window_get_frame_extents is not returning the correct position when o i3-wm, probably by some bug on the wm itself.

Te fix replaces the usage of the function for already known extents value calculation.

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)

Issue

JDK-8251240: Menus inaccessible on Linux with i3 wm (Bug - P4)

Reviewers

Jose Pereda (@jperedadnr - Reviewer)
Johan Vos (@johanvos - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1173

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2023-07-09T17:43:34Z

👋 Welcome back tsayao! 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.

mlbridge · 2023-07-09T17:47:57Z

Webrevs

00: Full (5dd57d4e)

jperedadnr

Looks good to me!
Tested on Ubuntu and Ubuntu with i3 window manager.
Passes on the latter after this patch, and fails before it.

openjdk · 2023-07-09T20:06:35Z

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

8251240: Menus inaccessible on Linux with i3 wm

Reviewed-by: jpereda, jvos

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

e8491ba: 8283675: Line not removed from LineChart when series cleared
84aad81: 8314064: Enable building JavaFX on native Windows AArch64 (ARM64)
82f2774: 8311097: Synchron XMLHttpRequest not receiving data
beca88c: 8262518: SwingNode.setContent does not close previous content, resulting in memory leak
53682dd: 8314589: javadoc build only shows the first 100 warnings and errors
be2c7ae: 8312058: Documentation improvements for subscription based listeners
6a7c743: 8274932: Render scales in EmbeddedWindow are not properly updated
2e937bb: 8313956: focusWithin on parents of a newly-added focused node is not updated
ddd1f79: 8314212: Crash when loading cnn.com in WebView
7b797b9: 8309558: Create implementation of NSAccessibilityCheckBox protocol
... and 34 more: https://git.openjdk.org/jfx/compare/5aad0406e5ab18d9da56e8adb4c013b93be1ba3d...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.

johanvos · 2023-07-10T08:06:34Z

modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp

    if (frame_type == TITLED) {
-        GdkRectangle rect;
-        gdk_window_get_frame_extents(gdk_window, &rect);


Looking at potential regression: in case of a TITLED frame, before this patch you would use get_frame_extents and now you will use get_origin in all cases. Are there scenario's where this might lead to regression, or is there a way to test this?

geometry.x and y should point to the top left corner of the window including decorations (on JavaFX logic), then the view position will have the decoration size as x and y. gdk_window_get_frame_extents returns the top left position with frame extents (decorations) - all other Gdk/Gtk functions will return position without decorations (because on X11 they are drawn on client side). Somehow gdk_window_get_frame_extents is broken on i3-wm, but we already have the frame extents values. I just replaced it by getting normal origins (does not include decorations) and if the frame is TITLED, subtract the frame extents to have the window position accounting decorations.

To test this I:

Run the bug example with UNDECORATED window on i3 and mutter (the default gnome wm);

Run the bug example with DECORATED window on i3 and mutter (the default gnome wm);

johanvos · 2023-07-10T08:08:44Z

/reviewers 2

openjdk · 2023-07-10T08:09:33Z

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

bridgekeeper · 2023-08-07T13:39:54Z

@tsayao 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 add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

tsayao · 2023-09-03T15:33:33Z

Can someone sponsor this?

johanvos

I am very careful when generic glass code is changed, but with the explanation added, I don't see a potential case for regression, so +1 from me.
We should encourage developers to use 22-ea builds to make sure there is indeed no regression.

kevinrushforth · 2023-09-15T12:50:11Z

@tsayao This is ready to integrate when you are ready.

tsayao · 2023-09-15T12:55:48Z

/integrate

openjdk · 2023-09-15T12:56:45Z

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

f7b21e5: 8315074: Possible null pointer access in native glass
7c8dd1e: 8315958: Missing range checks in GlassPasteboard
624fe86: 8313651: Add 'final' keyword to public property methods in controls
325be56: 8310666: gradle validateSourceSets task not run when TEST_ONLY=true
eb7de72: 8315317: Add test for JDK-8262518
ed92171: 8315870: icu fails to compile with Visual Studio 2022 17.6.5
8fcd6e5: 8308608: [testbug] Use Util::waitForIdle instead of Toolkit::firePulse in system tests
3bfede8: 8314779: [testbug] Add test to all the XYCharts to check if chart components are removed when series is cleared
668e4b9: 8315728: [testbug] SystemMenuBarTest prints "FAILED IS: false"
e8491ba: 8283675: Line not removed from LineChart when series cleared
... and 43 more: https://git.openjdk.org/jfx/compare/5aad0406e5ab18d9da56e8adb4c013b93be1ba3d...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-09-15T12:56:55Z

@tsayao Pushed as commit f185974.

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

tsayao added 2 commits July 9, 2023 14:36

Fix menu position on i3 wm

1d7e7d6

Merge branch 'master' into fix_menu_i3_wm

5dd57d4

openjdk bot added the rfr Ready for review label Jul 9, 2023

jperedadnr approved these changes Jul 9, 2023

View reviewed changes

openjdk bot added the ready Ready to be integrated label Jul 9, 2023

johanvos reviewed Jul 10, 2023

View reviewed changes

openjdk bot removed the ready Ready to be integrated label Jul 10, 2023

Siedlerchr mentioned this pull request Aug 5, 2023

Menus from the menu bar close immediately after left click is let go of if the menu bar was clicked in its top half JabRef/jabref#5867

Closed

2 tasks

johanvos approved these changes Sep 4, 2023

View reviewed changes

openjdk bot added the ready Ready to be integrated label Sep 4, 2023

openjdk bot added the integrated Pull request has been integrated label Sep 15, 2023

openjdk bot closed this Sep 15, 2023

openjdk bot removed ready Ready to be integrated rfr Ready for review labels Sep 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8251240: Menus inaccessible on Linux with i3 wm #1173

8251240: Menus inaccessible on Linux with i3 wm #1173

tsayao commented Jul 9, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jul 9, 2023

mlbridge bot commented Jul 9, 2023

jperedadnr left a comment

openjdk bot commented Jul 9, 2023 •

edited

Loading

johanvos Jul 10, 2023

tsayao Jul 10, 2023

johanvos commented Jul 10, 2023

openjdk bot commented Jul 10, 2023

bridgekeeper bot commented Aug 7, 2023

tsayao commented Sep 3, 2023

johanvos left a comment

kevinrushforth commented Sep 15, 2023

tsayao commented Sep 15, 2023

openjdk bot commented Sep 15, 2023

openjdk bot commented Sep 15, 2023

8251240: Menus inaccessible on Linux with i3 wm #1173

8251240: Menus inaccessible on Linux with i3 wm #1173

Conversation

tsayao commented Jul 9, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Jul 9, 2023

mlbridge bot commented Jul 9, 2023

Webrevs

jperedadnr left a comment

Choose a reason for hiding this comment

openjdk bot commented Jul 9, 2023 • edited Loading

johanvos Jul 10, 2023

Choose a reason for hiding this comment

tsayao Jul 10, 2023

Choose a reason for hiding this comment

johanvos commented Jul 10, 2023

openjdk bot commented Jul 10, 2023

bridgekeeper bot commented Aug 7, 2023

tsayao commented Sep 3, 2023

johanvos left a comment

Choose a reason for hiding this comment

kevinrushforth commented Sep 15, 2023

tsayao commented Sep 15, 2023

openjdk bot commented Sep 15, 2023

openjdk bot commented Sep 15, 2023

tsayao commented Jul 9, 2023 •

edited by openjdk bot

Loading

openjdk bot commented Jul 9, 2023 •

edited

Loading