Closed Bug 1807482 Opened 2 years ago Closed 2 years ago

sawfish window manager: pulldown menus disappear upon hovering (109.0)

Categories

(Core :: Widget: Gtk, defect)

Firefox 109
x86_64
Linux
defect

Tracking

()

VERIFIED FIXED
110 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox108 --- unaffected
firefox109 --- disabled
firefox110 --- verified

People

(Reporter: balducci, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/109.0

Steps to reproduce:

click (both left-down and left-up events) on a pull down menu (eg Bookmarks, but any other pull down menu will behave the same way): the pull down menu is displayed.

Actual results:

The pull down menu disappears as soon as I hover it

Expected results:

With firefox <=108 the menu stays displayed when hovering and I can select an item by clicking again on it

With 109, the only way to be able to select an item from the pull down is to
click on the menu label, but DO NOT RELEASE the mouse button: then
(still with the mouse button held down), I can select an item of the
menu by releasing the mouse button when on it

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Widget: Gtk
Product: Firefox → Core

I can't repro this. Can you run mozregression (see below) to see what broke this on your set up? Can you attach about:support?

$ pip3 install --user mozregression
$ mozregression --good 108 --bad 109
Flags: needinfo?(balducci)
Attached file about:support (deleted) —

hi
thanks for spending time on this

attaching about:support

I'll run mozregression as soon as possible

ciao
-g

... here is the bisection result:

11:45.23 INFO: No more integration revisions, bisection finished.
11:45.23 INFO: Last good revision: 850c9756f206a4e29a7b2720fad6c918ffda33df
11:45.23 INFO: First bad revision: 9b1c242dc2e0dce955e83320a28227c9817d9d57
11:45.23 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=850c9756f206a4e29a7b2720fad6c918ffda33df&tochange=9b1c242dc2e0dce955e83320a28227c9817d9d57

your availability is warmly appreciated

ciao
-g

Flags: needinfo?(balducci)

Thanks! Which desktop environment/compositor do you have?

Keywords: regression
OS: Unspecified → Linux
Regressed by: 1798131
Hardware: Unspecified → x86_64

no desktop/compositor
just startx and using sawfish window manager

Summary: 109.0: pulldown menus disappear upon hovering → sawfish window manager: pulldown menus disappear upon hovering (109.0)

based on changed summary (above) I have experimented with three wm I have at hand (all at the latest version) in the hope to be of some help:

window manager behavior
sawfish bad
twm bad
icewm good

(in all three tests I run firefox-109.0b6)

The bug has a release status flag that shows some version of Firefox is affected, thus it will be considered confirmed.

Status: UNCONFIRMED → NEW
Ever confirmed: true

Let me try to repro this.

Flags: needinfo?(emilio)

This is just preliminary clean-up.

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Flags: needinfo?(emilio)

Some X servers move focus along with the mouse. They do not do that if
the pointer is grabbed tho.

Since we want to rollup popups on focus-out events, and not grabbing the
pointer causes the main window to lose focus, let's try to grab the
pointer again.

This reintroduces similar code to the one removed in bug 1798131, but
with a couple critical differences:

  • It's simpler.

  • We use less-deprecated APIs, though we can't use GdkSeat because we
    still support Gtk < 3.20 in theory.

  • We don't capture enter/leave-notify events in the grab mask. This
    doesn't seem to be needed, and prevents the spurious
    enter/leave-notify events that caused me to remove the pointer grabs
    to begin with.

For now I reintroduced the grab on X11 whole-sale.

If we see back the intermittent failures from bug 1607713 we might want
to detect the environment in a more fine-grained fashion as suggested on
the code comment.

Depends on D165734

Keywords: leave-open

I wrote this while debugging test_panel_mouse_coords.xhtml for the
patch above.

Doesn't change behavior but since GetRefPoint can do WidgetToScreenOffset
which might be expensive, let's just avoid it.

Depends on D165735

Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e33eb418926a Re-introduce a reduced version of mouse grabs. r=stransky https://hg.mozilla.org/integration/autoland/rev/49b134c7ce20 Avoid computing refpoint multiple times. r=stransky

Backed out for causing failures on browser_bookmark_popup.js

[task 2023-01-04T02:23:24.803Z] 02:23:24     INFO - TEST-PASS | browser/components/places/tests/browser/browser_bookmark_popup.js | remove/cancel button label should match isEditingBookmark state - "bookmark-panel-cancel" == "bookmark-panel-cancel" - 
[task 2023-01-04T02:23:24.804Z] 02:23:24     INFO - Buffered messages finished
[task 2023-01-04T02:23:24.805Z] 02:23:24     INFO - TEST-UNEXPECTED-FAIL | browser/components/places/tests/browser/browser_bookmark_popup.js | Test timed out - 
[task 2023-01-04T02:28:28.587Z] 02:28:28     INFO - GECKO(1570) | console.error: (new Error("Invalid ETag value \"undefined\"", "resource://services-settings/SyncHistory.jsm", 52))
[task 2023-01-04T02:30:28.583Z] 02:30:28     INFO - GECKO(1570) | 1672799428581	addons.xpi	ERROR	System addon update list error Error: got node name: html, expected: updates
[task 2023-01-04T02:30:28.585Z] 02:30:28     INFO - Console message: [JavaScript Error: "1672799428581	addons.xpi	ERROR	System addon update list error Error: got node name: html, expected: updates" {file: "resource://gre/modules/Log.sys.mjs" line: 723}]
[task 2023-01-04T02:30:28.586Z] 02:30:28     INFO - append@resource://gre/modules/Log.sys.mjs:723:12
[task 2023-01-04T02:30:28.586Z] 02:30:28     INFO - log@resource://gre/modules/Log.sys.mjs:377:16
[task 2023-01-04T02:30:28.586Z] 02:30:28     INFO - error@resource://gre/modules/Log.sys.mjs:385:10
[task 2023-01-04T02:30:28.586Z] 02:30:28     INFO - updateSystemAddons/res<@resource://gre/modules/addons/XPIInstall.jsm:4076:25
[task 2023-01-04T02:30:28.586Z] 02:30:28     INFO - 
[task 2023-01-04T02:36:38.601Z] 02:36:38     INFO - Buffered messages finished
[task 2023-01-04T02:36:38.602Z] 02:36:38    ERROR - TEST-UNEXPECTED-TIMEOUT | browser/components/places/tests/browser/browser_bookmark_popup.js | application timed out after 370 seconds with no output
[task 2023-01-04T02:36:38.603Z] 02:36:38    ERROR - Force-terminating active process(es).

Also failing: /builds/worker/workspace/obj-build/dist/include/mozilla/Logging.h:290:61: error: format '%d' expects argument of type 'int', but argument 7 has type 'nsIWidget::NativeMouseMessage'-Werror=format=

Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/be489dde8ba3 Avoid computing refpoint multiple times. r=stransky
Attachment #9310234 - Attachment description: Bug 1807482 - Re-introduce a reduced version of mouse grabs. r=stransky → Bug 1807482 - Re-introduce a reduced version of mouse grabs for desktop environments that need it. r=stransky

Yeah, so this re-introduces the crashes from bug 1607713 (which were re-filed as bug 1808445). I'm on the verge of WONTFIX-ing this, but let's detect Sawfish / twm and do the grab only there as Firefox does look broken without it.

Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6b9b8523de07 Re-introduce a reduced version of mouse grabs for desktop environments that need it. r=stransky

.

Keywords: leave-open

Hi Emilio, do we have more work to do on this bug and are we still working to fix this for 109?

Flags: needinfo?(emilio)

Once the last patch lands on central it should get closed. We could consider uplifting I suppose.

Flags: needinfo?(emilio)
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 110 Branch

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox109 to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(emilio)

Comment on attachment 9310233 [details]
Bug 1807482 - Remove gRollupListener. r=stransky

Beta/Release Uplift Approval Request

  • User impact if declined: On some old window managers dropdown menus don't work.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: If QE can check those WMs it'd be great, otherwise it's probably not a huge deal.
  • List of other uplifts needed: none
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It only changes behavior for known-affected window managers.
  • String changes made/needed: none
  • Is Android affected?: No
Flags: needinfo?(emilio)
Attachment #9310233 - Flags: approval-mozilla-beta?
Attachment #9310234 - Flags: approval-mozilla-beta?
Attachment #9310526 - Flags: approval-mozilla-beta?

Comment on attachment 9310233 [details]
Bug 1807482 - Remove gRollupListener. r=stransky

Approved for 109.0b9 so we can avoid shipping this regression in 109.

Attachment #9310233 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9310234 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9310526 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Approved for 109.0b9 so we can avoid shipping this regression in 109

just confirming that 109.0b9 fixes the problem with sawfish for me

I have noticed, however, that twm still suffers it: not a big deal
for me as I use sawfish, but I thought it would have been good to
make you know...

thank you very much indeed for fixing this

ciao
-gabriele

(In reply to gabriele balducci from comment #30)

Approved for 109.0b9 so we can avoid shipping this regression in 109

just confirming that 109.0b9 fixes the problem with sawfish for me

I have noticed, however, that twm still suffers it: not a big deal
for me as I use sawfish, but I thought it would have been good to
make you know...

thank you very much indeed for fixing this

ciao
-gabriele

Thank you for verifying! I have tried to reproduce and verify the issue on my end with Firefox 109.0b6 after installing the sawfish window manager inside Ubuntu 20.04 and entering sawfish from the logging screen cogwheel but without any luck. The bookmarks dropdown menu is opened on the left click and remains displayed until a bookmark is selected. I have also tried to use twm but for some reason, I cannot open it (it returns me to the logging screen).
Gabriele, if you have the time could you please also try to verify the fix using the latest nightly on the windows managers you have available?
Also if the issue still reproduces on twm window manager, could you also file a new ticket for that as well? Thank you in advance!

Flags: needinfo?(balducci)

For what it's worth, the detection of twm here will probably only work if it's run from a display manager (like a graphical login screen), not if it's started via startx. twm doesn't seem to set the _NET_SUPPORTING_WM_CHECK property that we'd use to get the window manager's name, so Firefox's source for that information is an environment variable that a display manager would set. If using startx, doing something like export XDG_SESSION_DESKTOP=twm in .xinitrc should activate the workaround; I can reproduce this in twm and setting that env var is necessary for this patch to fix it.

But what's going on here seems to be a little more complicated. I'm not seeing exactly this bug with fvwm (using focus-follows-mouse), but something less severe: I can move the pointer into the menu, but if I move the pointer out of the menu into the main window then the menu is dismissed. This makes it a little annoying to select menu items, especially in submenus, but it's possible. Before bug 1798131 — or with this patch if and only if I set XDG_SESSION_DESKTOP=twm (even though it's not actually twm) — then menus require a click outside the menu or hitting the escape key to dismiss, matching the behavior of menus/dropdowns in other apps (e.g.: keepassxc (Qt), chromium (GTK?), pavucontrol (GTK)).

Interestingly, with openbox the menu isn't dismissed if the pointer moves from the menu into the browser window, but it is dismissed if it moves into another window. In contrast, the three other apps I mentioned grab focus as long as the menu is active, even if the pointer moves, so that doesn't apply to them.

I'm not actually clicking the mouse other than to open the menu — and opening the menu works normally — so I wouldn't expect the window manager bug found in bug 1805939 (which fvwm has but openbox doesn't) to apply here.

(In reply to Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧ from comment #33)

Interestingly, with openbox the menu isn't dismissed if the pointer moves from the menu into the browser window, but it is dismissed if it moves into another window.

Without a pointer-grab, a focus change is the point where Firefox knows that the user is now interacting with a different app. Popup windows are not managed by the window-manager, and so it is correct for the app to hide such windows when focus moves to another app. If it does not do so, then the popup would remain in the way of other managed windows even when they are raised.

bug 1807482, comment 31

Gabriele, if you have the time could you please also try to verify the fix using the latest nightly on the windows managers you have available?

I confirm that twm still suffers the popdown disappearance upon hovering with latest nightly (sawfish and icewm work fine)

bug 1807482, comment 32

I can reproduce this in twm and setting that env var is necessary for this patch to fix it

that doesn't work for me; I mean: using startx and setting export XDG_SESSION_DESKTOP=twm in .xinitrc does not fix popdown disappearance upon hovering when using twm

Flags: needinfo?(balducci)

Thank you Gabriele for verifying. I will close this as verified based on comment 35 and comment 30 since the issue is no longer reproducing on the sawfish window manager.

Given the fact that the twm window manager is still affected on your end by this issue can you please file a new ticket for this as well if possible? Thank you!

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+ → needinfo?(balducci)

This was backed out from 109 for the RC2 build to avoid shipping bug 1805939.
https://hg.mozilla.org/releases/mozilla-release/rev/d6f4c3448906

hi
in comment#35 above I reported that the fix mentioned in comment#32, ie exporting XDG_SESSION_DESKTOP=twm in .xinitrc, was not working for me

After some more experiments I have eventually realized I was doing a trivial error in my original test.

I was using a copy of my current .xinitrc modified as:

[...]
xterm -geometry 80x23+0+0 &
export XDG_SESSION_DESKTOP=twm
xterm -e twm

and running firefox from the xterm launched from .xinitrc, which, obviously, didn't have any XDG_SESSION_DESKTOP environment variable set. After changing to:

[...]
export XDG_SESSION_DESKTOP=twm
xterm -geometry 80x23+0+0 &
xterm -e twm

(or running firefox from an xterm freshly started from the twm menu) I do indeed reproduce the XDG_SESSION_DESKTOP=twm fix for twm

In conclusion, for me, twm behavior is also fixed, once (correctly!) exporting XDG_SESSION_DESKTOP=twm to the environment, as mentioned in comment#32 above

Apologies for the additional noise...

Flags: needinfo?(balducci)
Regressions: 1811031
No longer regressions: 1811031
Regressions: 1813498
Duplicate of this bug: 1816364
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: