1899299 - [Thunderbird] With Linux, Dropdowns in Message Filter Rules don't close when clicking outside

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P3)

Description

[Alex Franchuk [:afranchuk]]

This is likely a GUI implementation issue, maybe something to do with a window click event handler being overriden. The select box dropdowns in other windows close when clicking elsewhere in the window, however in the Filter Rules window all select box dropdowns (whether the "Getting New Mail: [select box]" or those in the filter rules/actions) never close. They only close when I move the mouse out of the window entirely.

I'm using X11 with XFCE4.

Comment 1

[Magnus Melin [:mkmelin]]

I see this too on Ubuntu with Gnome, on daily.
STR:
Tools | Filters
New...
In the rules, click open the rule to choose Subject or something else
Try to click outside.

Actual result: popup doesn't close. (It does close when clicking the popup itself.)
Expected: should close the popup when clicking elsewhere in the dialog.

This wasn't an issue in 115.

Alice, can we get a regression window?

Comment 2

[Alice0775 White]

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=d11ce652f1e6fff06e8c4de52af63e483d4dd8b4&tochange=7cba2f9bbb38c538d77084a68414fe32d4bd8478
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=192e73e7c91313c09603efd1650931b917746bf4&tochange=3ddab45ce6bdbc0501b54ae64cbef202e8f92fb5

Suspect: Bug 1749174 - Enable wayland on supported environments by default on Nightly / Early Beta.

Regression window even if export MOZ_ENABLE_WAYLAND=0 :
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=07b121b1f4d828b99d6e00ab04c65c73ee141de4&tochange=c1975399a306d335b707ca9dc17d44e698c8e827
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a137631f73ce188671f50c5225295d64d427fec2&tochange=2ff68fa9f49916065f4ebb7a74c0940bf425f396

Suspect: Bug 1864382

Comment 3

[Magnus Melin [:mkmelin]]

Thanks Alice!

Martin, Emilio, any ideas?

Comment 4

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

Can you create a screencast of the issue?
https://fedoraproject.org/wiki/How_to_debug_Firefox_problems#Create_screenshot/screencast_for_a_bug_report
Thanks.

Comment 5

[Magnus Melin [:mkmelin]]

Kind of, since it's really about nothing happening on clicks outside the dropdown, the video doesn't show the issue too much.

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: Screencast from 2024-06-05 16-04-10.webm

Pretend that after opening the dropdown, I click around in the dialog (nothing happens: this is the bug).
Then i click inside the dropdown and it closes (like it should).

Comment 7

[Alex Franchuk [:afranchuk]]

Yeah, mkmelin's screencast represents the issue as best as possible.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Yeah, so... We somehow don't seem to get those button presses at all from GTK. Same for other pointer events. You can see this with the cursor not changing when hovering over stuff.

If I make the code listen to mShell rather than mContainer, then it works:

diff --git a/widget/gtk/nsWindow.cpp b/widget/gtk/nsWindow.cpp
index 297149796986c..b007959812381 100644
--- a/widget/gtk/nsWindow.cpp
+++ b/widget/gtk/nsWindow.cpp
@@ -6332,23 +6332,23 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
   // and propagate up to the first widget that handles the events, so we
   // need only connect on mShell, if it exists, to catch events on its
   // window and windows of mContainer.
-  g_signal_connect(mContainer, "enter-notify-event",
+  g_signal_connect(mShell, "enter-notify-event",
                    G_CALLBACK(enter_notify_event_cb), nullptr);
-  g_signal_connect(mContainer, "leave-notify-event",
+  g_signal_connect(mShell, "leave-notify-event",
                    G_CALLBACK(leave_notify_event_cb), nullptr);
-  g_signal_connect(mContainer, "motion-notify-event",
+  g_signal_connect(mShell, "motion-notify-event",
                    G_CALLBACK(motion_notify_event_cb), nullptr);
-  g_signal_connect(mContainer, "button-press-event",
+  g_signal_connect(mShell, "button-press-event",
                    G_CALLBACK(button_press_event_cb), nullptr);
-  g_signal_connect(mContainer, "button-release-event",
+  g_signal_connect(mShell, "button-release-event",
                    G_CALLBACK(button_release_event_cb), nullptr);
-  g_signal_connect(mContainer, "scroll-event", G_CALLBACK(scroll_event_cb),
+  g_signal_connect(mShell, "scroll-event", G_CALLBACK(scroll_event_cb),
                    nullptr);
   if (gtk_check_version(3, 18, 0) == nullptr) {
-    g_signal_connect(mContainer, "event", G_CALLBACK(generic_event_cb),
+    g_signal_connect(mShell, "event", G_CALLBACK(generic_event_cb),
                      nullptr);
   }
-  g_signal_connect(mContainer, "touch-event", G_CALLBACK(touch_event_cb),
+  g_signal_connect(mShell, "touch-event", G_CALLBACK(touch_event_cb),
                    nullptr);

   LOG("  nsWindow type %d %s\n", int(mWindowType),

Martin, do you know what might be going on / why gtk isn't sending us those events? Alternatively do you know of any issue if we just listen to events on mShell consistently?

Comment 9

[Hartmut Welpmann [:welpy-cw]]

An apparently related issue has been reported in bug 1902013. For that I came up with the same regression window as in comment 2 (second one) with Debian/XFCE, even though the reporter there uses 115.11.0.

Comment 10

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

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

Martin, do you know what might be going on / why gtk isn't sending us those events? Alternatively do you know of any issue if we just listen to events on mShell consistently?

I'd say there's some event routing involved, perhaps the widget is created as input transparent or so? Is that reproducible with Thunderbird only? Do we see that in Firefox anywhere? I wonder how the popup is created and configured.

Can you please run TB with MOZ_LOG="Widget:5 WidgetPopup:5" env variable on terminal, reproduce it and attach the log here?
Thanks.

Comment 11

[Alex Franchuk [:afranchuk]]

Attachment: 1899299-widgets.log

Here's the log. For your convenience, I interjected "STARTING" and "ENDED" lines around my interaction with the filter dialog dropdowns (though I include the entire log in case there's something else pertinent there). This captures the log of me opening the filters dialog, clicking "new", opening a single dropdown and then clicking around in the window (with the dropdown not closing and clicks doing nothing) until I move the mouse out of the window (at which point the dropdown closes).

Comment 12

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

Thanks. The whole sequence of menu creation look weird, I see there are many hidden menu popups opened/closed with the same parent window. I'd expect there's only two windows used - main dialog and the dropdown menu.

[Parent 24458: Main Thread]: D/Widget [7fe0b0b28c00]: nsWindow::Create
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b0b28c00]:   mBounds: x:0 y:0 w:0 h:0
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b0b28c00]: nsWindow::Create() Popup
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b0b28c00]: nsWindow::Create() popup type Menu
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b0b28c00]:     set parent window [7fe0ac469600] filtereditor

[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b0b29100]: nsWindow::Create() Popup
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b0b29100]: nsWindow::Create() popup type Menu
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b0b29100]:     set parent window [7fe0ac469600] filtereditor

[Parent 24458: Main Thread]: D/Widget [7fe0b392c600]: nsWindow::Create
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b392c600]: nsWindow::Create() Popup
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b392c600]: nsWindow::Create() popup type Menu
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b392c600]:     set parent window [7fe0ac469600] filtereditor

[Parent 24458: Main Thread]: D/Widget [7fe0b3a3d200]: nsWindow::Create
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a3d200]:   mBounds: x:0 y:0 w:0 h:0
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a3d200]: nsWindow::Create() Popup
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a3d200]: nsWindow::Create() popup type Menu
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a3d200]:     set parent window [7fe0ac469600] filtereditor

[Parent 24458: Main Thread]: D/Widget [7fe0b0b29600]: nsWindow::Create
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b0b29600]:   mBounds: x:0 y:0 w:0 h:0
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b0b29600]: nsWindow::Create() Popup
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b0b29600]: nsWindow::Create() popup type Menu
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b0b29600]:     set parent window [7fe0ac469600] filtereditor

[Parent 24458: Main Thread]: D/Widget [7fe0b3a83e00]: nsWindow::Create
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83e00]:   mBounds: x:0 y:0 w:0 h:0
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83e00]: nsWindow::Create() Popup
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83e00]: nsWindow::Create() popup type Menu
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83e00]:     set parent window [7fe0ac469600] filtereditor

[Parent 24458: Main Thread]: D/Widget [7fe0b3a83400]: nsWindow::Create
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83400]:   mBounds: x:0 y:0 w:0 h:0
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83400]: nsWindow::Create() Popup
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83400]: nsWindow::Create() popup type Menu
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83400]:     set parent window [7fe0ac469600] filtereditor

[Parent 24458: Main Thread]: D/Widget [7fe0b3a83900]: nsWindow::Create
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83900]:   mBounds: x:0 y:0 w:0 h:0
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83900]: nsWindow::Create() Popup
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83900]: nsWindow::Create() popup type Menu
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3a83900]:     set parent window [7fe0ac469600] filtereditor

[Parent 24458: Main Thread]: D/Widget [7fe0b3fc1b00]: nsWindow::Create
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3fc1b00]:   mBounds: x:0 y:0 w:0 h:0
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3fc1b00]: nsWindow::Create() Popup
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3fc1b00]: nsWindow::Create() popup type Menu
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3fc1b00]:     set parent window [7fe0ac469600] filtereditor

arent 24458: Main Thread]: D/Widget [7fe0b3fc1100]: nsWindow::Create
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3fc1100]:   mBounds: x:0 y:0 w:0 h:0
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3fc1100]: nsWindow::Create() Popup
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3fc1100]: nsWindow::Create() popup type Menu
[Parent 24458: Main Thread]: D/WidgetPopup [7fe0b3fc1100]:     set parent window [7fe0ac469600] filtereditor

Comment 13

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

Magnus, can you point me where the filter dialog is located? Looks like TB is creating bogus popups which confuses the widget code.
Thanks.

Comment 14

[Magnus Melin [:mkmelin]]

Sure,
FilterEditorDialog: https://searchfox.org/comm-central/source/mailnews/search/content/FilterEditor.xhtml
The popup with the reported issue is created here: https://searchfox.org/comm-central/rev/a7f02aed18d176997ab8a762790a8b7d49d64aff/mailnews/search/content/searchWidgets.js#284 as part of a <search-operator> custom element - https://searchfox.org/comm-central/source/mailnews/search/content/searchWidgets.js#507

Comment 15

[Magnus Melin [:mkmelin]]

FilterEditorDialog is opened by code from another dialog, https://searchfox.org/comm-central/source/mail/base/content/FilterListDialog.xhtml

Comment 16

[Alex Franchuk [:afranchuk]]

Does "Wayland" in the bug title imply that this is a wayland-only issue? I'm not running wayland.

Comment 17

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

(In reply to Alex Franchuk [:afranchuk] from comment #16)

Does "Wayland" in the bug title imply that this is a wayland-only issue? I'm not running wayland.

Yes, I removed it.

Comment 19

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

Fixed when reverted https://phabricator.services.mozilla.com/D193401 (but still broken on Wayland anyway).
We created popups without CSD and always painted to mShell before D193401 so the patch from comment 8 was active for popups.

Comment 21

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

We're hitting more bugs together here:

• We create the popup as modal, it means underlying dialog (also modal) doesn't get mouse button events so we don't rollup the popup. We did that due to missing mouse grab (or its different architecture at Wayland so we can't use it).

• If we get modal popup click outside, we receive such event on mShell and not on mContainer (why? Perhaps due to the modal state?)

• If we connect event handlers to popup mShell (as we had before), due to bug (or feature) at GetFirstNSWindowForGDKWindow() we wrongly re-direct events from popup's mShell to parent dialog nsWindow so the popup is closed.

As due to missing pointer grab we need to emulate it with modal popup. I guess our only way is to detect outside click as mouse button event between leave - enter events, i.e. mouse button click when cursor is outside the popup.

Comment 22

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

Note that the bug was here before and changes at Bug 1864382 only reveals it.

Comment 23

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

Attachment: Bug 1899299 [Linux] Remove mRetryPointerGrab as it's unused r?emilio

Comment 24

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

Attachment: Bug 1899299 [Linux] Remove GetFirstNSWindowForGDKWindow() as we don't use widget plugins r?emilio

Depends on D216911

Comment 25

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

Attachment: Bug 1899299 [Linux] Move some widget events from mContainer to mShell r?emilio

• Move more widget events from mContainer to mShell. We generally want to listen on mShell and use mContainer
  for GtkContainer related events only.
• Set correct event masks for mShell/mContainer

Depends on D216912

Comment 26

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

Attachment: Bug 1899299 [Linux] Reformat widget event code r?emilio

Depends on D216913

Comment 27

[Geoff Lankow (:darktrojan)]

Attachment: simplescreenrecorder-2024-07-18_20.53.40.mp4

Here's a screen recording of the developer toolbox with the changes. The inspector view should have a hover effect as the mouse rolls over it, but it doesn't. Clicking on things doesn't work either.

Comment 28

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

(In reply to Geoff Lankow (:darktrojan) from comment #27)

Created attachment 9413460 [details]
simplescreenrecorder-2024-07-18_20.53.40.mp4

Here's a screen recording of the developer toolbox with the changes. The inspector view should have a hover effect as the mouse rolls over it, but it doesn't. Clicking on things doesn't work either.

As per discussion it's MINT/X11. Will try that, I don't see it on Gnome.
There's also a regression that system window titlebar doesn't receive mouse clicks on Wayland.

Comment 29

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

Attachment: Bug 1899299 [Linux] Keep propagating relocated mShell events to child widgets r?emilio

Depends on D216914

Comment 30

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

Geoff, please try with D216914.
Thanks.

Comment 31

[Geoff Lankow (:darktrojan)]

Attachment: simplescreenrecorder-2024-07-19_12.28.23.mp4

I can see what the problem might be, and it might something for a different bug. When the toolbox window is docked to the right hand side of the screen, the mouse events are happening as though the window was still in its previous position. In this recording you can see the mouse moving on the left side of the window, but things are happening on the right side of the window.

This doesn't seem to happen every time the window is docked, so I'm not sure what the trigger is. I was able to reproduce it fairly consistently in Firefox:

• Run ./mach run --jsdebugger.
• Dock the devtools window with Super + Right Arrow.
• Close Firefox.
• Repeat the same steps. (The first time is just to set the initial size of the window for the second time.)

(This was recorded before I updated my copy of the patches, but it still happens with the latest version.)

Comment 32

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

(In reply to Geoff Lankow (:darktrojan) from comment #31)

(This was recorded before I updated my copy of the patches, but it still happens with the latest version.)

Do I understand correctly that it doesn't depends of the patches here?
Thanks.

Comment 33

[Geoff Lankow (:darktrojan)]

It only happens with the patches here.

Comment 34

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

Does D216914 make any difference here? Is the mouse motion bug less visible with it?
Thanks.

Comment 35

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

Attachment: WIP: Bug 1899299 [Linux] Attach more events to mContainer

Comment 36

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

Geoff, please test with D217163 on top. It adds signal handlers to mContainer.

Comment 37

[Geoff Lankow (:darktrojan)]

Yes, D217163 appears to have fixed it. Thanks!

Comment 38

[Geoff Lankow (:darktrojan)]

Attachment: Screenshot from 2024-07-23 20-42-08.png

I've just noticed that pop-ups in the developer toolbox don't pop up in the right place. Same STR as before. Like before, they're where the window used to be.

Comment 39

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

Lets' track possible regressions in another bugs.

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0c9c1dcb19c
[Linux] Remove mRetryPointerGrab as it's unused r=emilio
https://hg.mozilla.org/integration/autoland/rev/d6e1dbfd1002
[Linux] Remove GetFirstNSWindowForGDKWindow() as we don't use widget plugins r=emilio
https://hg.mozilla.org/integration/autoland/rev/d300cc8b778e
[Linux] Move some widget events from mContainer to mShell r=emilio
https://hg.mozilla.org/integration/autoland/rev/e0e1f122e646
[Linux] Reformat widget event code r=emilio
https://hg.mozilla.org/integration/autoland/rev/25389422d457
[Linux] Keep propagating relocated mShell events to child widgets r=emilio

Comment 41

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/a0c9c1dcb19c
https://hg.mozilla.org/mozilla-central/rev/d6e1dbfd1002
https://hg.mozilla.org/mozilla-central/rev/d300cc8b778e
https://hg.mozilla.org/mozilla-central/rev/e0e1f122e646
https://hg.mozilla.org/mozilla-central/rev/25389422d457

Comment 42

[Iulian Moraru]

Backed out for causing multiple functional regressions on Linux.

• Push that was backed out

• Backout link

Comment 43

[Cosmin Sabou [:CosminS]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/9b2d26f0ef7d

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c1b5657b7f1
[Linux] Remove mRetryPointerGrab as it's unused r=emilio
https://hg.mozilla.org/integration/autoland/rev/44257ccff494
[Linux] Remove GetFirstNSWindowForGDKWindow() as we don't use widget plugins r=emilio

Comment 45

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/3c1b5657b7f1
https://hg.mozilla.org/mozilla-central/rev/44257ccff494

Comment 46

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

Looks like the fix here is a part of the regression. When we register leave events to mShell we're getting leave events even for child popups which fixes the bug here but also brings new regression - but on X11 only.

Comment 47

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

Attachment: Bug 1899299 [Linux] Listen mouse events on mShell for modal popups r?emilio

A popup attached to a modal parent window doesn't get mouse events
from Gtk as they'are directed to the modal parent. This is usually solved
by pointer grab which that doesn't work on Wayland in our current
setup as it performs show and grab in one step.

We emulate it by setting popup as modal too but then patent
window doesn't get mouse events outside of popup (Bug 1899299).
we need to listen

Surprisingly attaching events handler to mShell fixes it
and we're getting events from both parent and popup windows.

Comment 48

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0727ff9372dc
[Linux] Listen mouse events on mShell for modal popups r=emilio

Comment 49

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/0727ff9372dc

Comment 50

[Dianna Smith [:diannaS]]

:stransky is this still supposed to be leave-open after that last patch that landed?

Comment 51

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

Should be fixed now, Thanks.

Comment 52

[Ryan VanderMeulen [:RyanVM]]

If this is causing problems for TB, we probably want to get this uplifted to ESR128 also? The 3 patches graft cleanly if you're comfortable nominating them for approval.

Comment 53

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

Let's wait to release if we see any issue on minor window managers.

Comment 55

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

Comment on attachment 9423099 [details]
Bug 1899299 [Linux] Listen mouse events on mShell for modal popups r?emilio

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Popups attached to modal windows can't be dismissed by clicking outside. It affects desktop background settings dialog in Firefox or Thunderbird message filters.
• User impact if declined: See above.
• Fix Landed on Version: 132
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The changes to window handling is always tricky as there are various minor window managers which may be affected by the change. But we don't have any related bug reports so far so let's consider it safe enough.

Comment 56

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

We need the last patch only to uplift.

Comment 58

[Donal Meehan [:dmeehan]]

Comment on attachment 9423099 [details]
Bug 1899299 [Linux] Listen mouse events on mShell for modal popups r?emilio

Approved for 128.5esr.

Comment 59

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/aefd43c367d0