fix: menus on Linux after window modification #37798
Conversation
|
Updated to disable the test on CI, unfortunately. While SSH'd in to test workarounds for |
dsanders11
added
target/22-x-y
| Window WindowCache::GetWindowAtPoint(gfx::Point point_px, | ||
| Window window, | ||
| const base::flat_set<Window>* ignore) { | ||
| - delete_when_destroy_timer_fires_ = true; |
There was a problem hiding this comment.
It seems that removing this line alone should be enough to fix the issue?
I'm good with a large patch reverting the commit, I'm just a bit worried it would take too much time for Chromium to fix the issue and this patch will become a burden on us.
There was a problem hiding this comment.
That's a good concern to call out.
It looks like they've fixed it pretty quickly upstream (although I haven't tested it in Electron): https://chromium-review.googlesource.com/c/chromium/src/+/4401194
So this fix could be changed to backport that upstream fix rather than doing a revert, although I think more rigorous testing should be done for that.
I think there might be more risk in backporting the upstream fix since it is a more general change which may be making assumptions/rely on other changes which have been made in upstream - I'd be concerned it might cause other subtle bugs by backporting it too far back. The revert in this PR is self-contained (AFAIK).
So I guess I'd say now that it's been fixed upstream, we can land this revert without too much burden on us, and we'll pick up the upstream fix in Electron v26.
|
Release Notes Persisted
|
|
I have automatically backported this PR to "25-x-y", please check out #37905 |
trop
bot
added
in-flight/25-x-y
and removed
target/25-x-y
|
I have automatically backported this PR to "24-x-y", please check out #37906 |
|
I have automatically backported this PR to "22-x-y", please check out #37907 |
|
I have automatically backported this PR to "23-x-y", please check out #37908 |
trop
bot
added
in-flight/22-x-y
and removed
target/24-x-y
trop
bot
added
merged/25-x-y
trop
bot
added
merged/24-x-y
Description of Change
Fixes #35724.
More details on that issue, but long and short of it is Chromium made a performance improvement change to the X11 window cache and that caused a bug with menus on Linux in Electron since v19.0.0-alpha.4. Seems safe to revert since it only aimed to improve performance and had no other functional changes. Upstream bug opened so they can properly fix it.
I don't love the test since it has arbitrary waits, but it's a tricky issue to test. You can confirm the added test fails on the latest nightly by using
e test --runners=main --electronVersion=25.0.0-nightly.20230331 -g "does not trigger issue #35724$".Checklist
npm testpassesRelease Notes
Notes: Fixed an issue on Linux where menus would not open after resizing/maximizing/unmaximizing a window