1727709 - [wayland] Webextension popups reportedly not being clickable in some cases after bug 1725604

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

See bug 1725604 comment 9.

I haven't been able to reproduce locally (yet at least). Viktor, what configuration are you using? (Distro, DE, WM, ...). Thanks.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

I couldn't repro with privacy badger either. :/

Does it happen only the first time you open the popup? Or when you reopen as well?

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1725604

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Attachment: Screencast of the issue

I can reproduce it, there's a screencast of the issue. It's reproducible only when a popup is opened for first time - when it's reopened the input area is correct.

Fedora 34 / Gnome / latest trunk.

Comment 4

[Martin Stránský [:stransky] (ni? me)]

Can it be caused by wrong mMouseTransparent state when popup is created? nsMenuPopupFrame::SetPopupState(nsPopupState aState) may apply the wrong mMouseTransparent state then.

Comment 5

[Martin Stránský [:stransky] (ni? me)]

We see https://gitlab.gnome.org/GNOME/gtk/-/issues/4166 here - when popup is moved by move-to-rect (i.e. wayland re-position it) the window needs to be hidden/show before the move as xdg_positioner does not work with visible windows.

Comment 6

[Anan Laks [:vulcansphere]]

I can reproduce this regression on Firefox Nightly Wayland in KDE Plasma 5.

Comment 7

[Viktor Jägersküpper]

I'm using Gnome Shell 40.4 with Mutter 40.4 on Arch Linux. When it happens (popup not clickable) and I close the popup by clicking on its icon or somewhere else, it can happen again, e.g. in the next try. The behaviour is different for every extension, Privacy Badger being the worst so far.

Comment 8

[Pascal]

Also running into this with sway. Clicks on extension elements go through to the content below. Mouse-over and right click are able to highlight an element with some delay though the latter also opens the context menu for the content below only. Could not get a single click to work (tested with uBlock Origin, KeepassXC, NordVPN, SearchByImage, SixOrNot, CleearURLs).

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Martin, is this bug impacting a lot of redhat users? (this should be prioritized and decided whether to fix in 93)
Thank you!

Comment 10

[Emilio Cobos Álvarez (:emilio)]

We should definitely fix this.

It's not clear to me if this is due to the gtk move resize issue mentioned above or due to the bit being out of sync as comment 4 suggests. But the bit is redundant anyways so we can remove it and it should fix this.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1727709 - Simplify nsMenuPopupFrame mouse transparent handling. r=stransky

Make sure it's always in sync with the style.

Keeping a boolean field was useful when we didn't handle dynamic changes
to it, but now we do it's just redundant, and could cause correctness
issues.

Comment 12

[Val Packett]

Sadly this patch doesn't seem to fix the issue. Clicks still go through.

Comment 13

[Martin Stránský [:stransky] (ni? me)]

While popup is moved by move-to-rect we need to hide/show the popup. I think when the popup is shown again the transparent state is deleted although we think it's still set.

I think we need to set correct mouse transparent state again at nsWindow::ShowWaylandWindow() where wayland window is shown
(after Bug 1727836).

Comment 14

[Martin Stránský [:stransky] (ni? me)]

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #9)

Martin, is this bug impacting a lot of redhat users? (this should be prioritized and decided whether to fix in 93)
Thank you!

Jan, Firefox/Wayland is used by default on all Fedoras so yes, we should fix that or revert for FF93.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

(In reply to Martin Stránský [:stransky] (ni? me) from comment #13)

While popup is moved by move-to-rect we need to hide/show the popup. I think when the popup is shown again the transparent state is deleted although we think it's still set.

Right, but given the default is "not transparent", shouldn't that cause clicks to go the the popup? I still haven't been able to repro :/

Anyhow I think the cleanup I posted above should be worth landing (even if we land it in a separate bug or tag the bug as leave-open).

Comment 16

[Martin Stránský [:stransky] (ni? me)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)

(In reply to Martin Stránský [:stransky] (ni? me) from comment #13)

While popup is moved by move-to-rect we need to hide/show the popup. I think when the popup is shown again the transparent state is deleted although we think it's still set.

Right, but given the default is "not transparent", shouldn't that cause clicks to go the the popup? I still haven't been able to repro :/

Hm, you're right.

From the debugging I did it looks like we set "transparent" by default for all popups and then "not transparent" later. I think the not transparent is propagated incorrectly somehow, maybe there's a race condition when we hide/show the popup.

Anyhow I think the cleanup I posted above should be worth landing (even if we land it in a separate bug or tag the bug as leave-open).

Sure, will look at it today.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bc363a25b712
Simplify nsMenuPopupFrame mouse transparent handling. r=stransky

Comment 18

[Martin Stránský [:stransky] (ni? me)]

I'm going to look at it since I can reproduce it.

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Thanks Martin!

Comment 20

[Emilio Cobos Álvarez (:emilio)]

(Worst case we can revert / soft-revert bug 1725604 for 93 or something)

Comment 21

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1727709 [Wayland] Force GdkWindow repaint after gdk_window_input_shape_combine_region() call, r?emilio

On Wayland gdk_window_input_shape_combine_region() call is cached and applied to underlying wl_surface when GdkWindow is repainted.
Force repaint of GdkWindow at nsWindow::SetWindowMouseTransparent() to apply the changes immediately.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec87b39f4cad
[Wayland] Force GdkWindow repaint after gdk_window_input_shape_combine_region() call, r=emilio

Comment 23

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/bc363a25b712

Comment 24

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/ec87b39f4cad