search

seanpringle / goomwwm

Get out of my way, Window Manager!
MIT License
190 stars 19 forks source link

Menus in recent (>=55) Chrome versions don't work #32

Open tomnomnom opened 7 years ago

tomnomnom commented 7 years ago

Hi there,

As of Chrome version 55 the menus in Chrome itself (<select> dropdowns, bookmarks menu, right click etc) do not work.

I consider this a Chrome bug, and I've filed this as a bug in Chromium already, but it's such a large and complex codebase that I haven't been able to figure out what the issue is myself.

You can see the bug I've raised here: https://bugs.chromium.org/p/chromium/issues/detail?id=703689

I narrowed the problem down to within a few revisions that there were downloads available for, and think this is most likely the commit that broke things: https://chromium.googlesource.com/chromium/src/+/3b6aeffa075816129e32e57412b1c89ba619df10

I don't know enough about C, C++, or X11 to diagnose the issue - and the response on the Chromium project has been slow; so I was hoping you might be able to at least give me a hint as to what might be causing the problem. I feel like the goomwwm code is simple enough that I might actually stand a chance of coming up with a workaround given a few pointers.

There's a video of the problem occurring on comment #6 on the Chromium bug report.

Any help would be greatly appreciated - I'm stuck between using an outdated browser version (which sucks for my job), and changing my window manager (which I really don't want to do - goomwwm is awesome).

seanpringle commented 7 years ago

Interesting -- I have reproduced this with Chromium 55.0.2883.87, but it's intermittent. Menus start off working fine, then suddenly stop appearing, then eventually (minutes++ later) return to normal. Repeat.

From the WM POV these events are happening:

Looks like the menu is created, mapped, then immediately closed for some reason. Since it's created with override_redirect=true it's not up to WM to do anything with it, and we don't...

Obviously the browser is expecting something else. Some race/timing condition perhaps.

[1] https://tronche.com/gui/x/xlib/events/window-state-change/map.html

tomnomnom commented 7 years ago

Thanks for that! I've posted a link to your comment on the Chromium bug in the hopes that it might help someone figure out what's going on.

I've tried a few other window managers and found them all to work fine. I've tested in awesomewm, xmonad, unity, xfce, and gnome.

At this point I'm not sure if it's a case of "Chrome is doing something weird and most window managers account for it", or "Chrome is doing something a bit unusual that goomwwm should be accounting for" - if that makes sense.

Looking at the diff for the suspected commit to chromium there are some changes relating to UnmapNotify, which increases my confidence that I'm right about which commit breaks things.

Are the events the same under Chrome 54? (or how can I figure out what events are happening?)

I've been trying to download and build the source for Chromium to help with debugging, but the >30gb of downloads and many hours of building is slow going on my connection and system :/

seanpringle commented 7 years ago

Probably the browser just being a bit unusual and we have to account for it. Chrome makes menus children of the root window and doesn't do helpful things like set WM_TRANSIENT_FOR which is a bit annoying, but Goomwwm's relatively strict tiling behaviour is sometimes a bit orthogonal to EWMH too.

Have managed to get something repeatable on a Ubuntu 16.04 system by:

In the last step re-opening a closed tab with ctrl+shift+t also gets the menus going again. However just opening a new tab or entering a new URL on an existing tab has no effect.

I don't have a Chrome 54 instance handy right now -- away from my main dev environment on a slow connection for a few days.

I generally debug X events with printf() in the WM. I've heard that xev/xinput can help but I've never seen a way to make those reliably listen to all events, and they're especially tricky to use with short-lived windows.

tomnomnom commented 7 years ago

I've been playing around with this some more. I'm not getting very far, but I've found a few bits and pieces.

dwm looks to have had a similar problem with Mathematica

I've checked that dwm works with the newer versions of chrome (which it does), but sadly it also works when I comment out the fix for the Mathematica problem and re-compile. So that might be a red herring.

I've also tried running chrome without a window manager at all, by creating an xsession .desktop file like this:

[Desktop Entry]
Type=Application
Exec=google-chrome-beta
Name=Chrome Beta
Comment=Testing

Everything works fine without a window manager. I can only assume therefore that goomwwm is doing something that interferes with the menus.

I'm very slowly learning about X, but it's a bit of an uphill struggle if I'm honest.

What I find interesting is that you sometimes have the menus work, but for me they never work; which leads me to believe you may be right about a race condition :(

seanpringle commented 7 years ago

I've found a fix for this in 1b84bb6cdc66a72279db24fb7d8d3b0c9df8cc15 .

However, it's odd... Goomwwm has (for years) respected WM_HINTS [1] set by applications, specifically InputHint which indicates whether a window wants the input focus forced upon it by the Window Manager using XSetInputFocus, or whether it prefers an async WM_TAKE_FOCUS message to handle focus transition itself. This is a necessary distinction for some types of windows, though many apps don't really care and can handle either approach.

By ignoring InputHint in this patch -- for the browser window, since the menus are override_redirect=1 and not explicitly focused by the WM regardless -- Chrome's menus start to work normally again on all my test boxes.

I don't think this is a direct causal relationship -- rather, I suspect that by removing the possibility of ever controlling focus using an async WM_TAKE_FOCUS event, we're actually avoiding some race condition in Chrome that Goomwwm's style of tiling and focus control happens to trigger.

I wonder if the core problem is something like:

</guess>

This patch may break other apps out there, hence it's not in master.

I have no clue why Chrome is so touchy about this stuff. Many other apps have override_redirect set for menus and work just fine in various floating/tiling/focus models.

It would be nice if the Chrome devs set a WM_CLASS on everything so that people could script around the strange design choices.

[1] https://tronche.com/gui/x/xlib/ICC/client-to-window-manager/wm-hints.html [2] https://tronche.com/gui/x/xlib/input/XSetInputFocus.html

tomnomnom commented 7 years ago

@seanpringle: you are amazing :)

I've switched to running off the chrome branch and Chrome 59 is working flawlessly so far.

I'll keep an eye out for any issues with other apps and let you know what I find. A quick run through of my most used apps hasn't turned anything up so far though!

tomnomnom commented 7 years ago

@seanpringle I've been running this build as my daily-driver on 2 different machines for nearly 2 months now and I've spotted zero issues. I'd say it's safe to merge :)

shashurup commented 5 years ago

Context menu in Vivaldi (Chrome based), main menu in SkypeForLinux and context menu in Slack don't work. All this seems to be browser engine based.

P.S. Goomwwm is really amazing! Never thought that there is still place for improvement in the area. The idea of predefined set of "reasonable" window sizes and places is really strong. Thank you for your work!

shashurup commented 5 years ago

Aha, sorry, I haven't noticed that the fix is in "chrome" branch, not in master.