1645695 - [Wayland] Hamburger menu can't be closed by sequent mouse click when repositioned by screen limits

Status: RESOLVED WORKSFORME

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

Description

[Darkspirit]

Gnome Wayland, Debian Testing, Macbook Pro, 2560x1600, 200% scaling
The main menu is affected by multiple regressions.

Maximized window:
With a maximized window, the menu panel was often misplaced and covering the menu button, so it's hard to test.
Often, the maximized window itself was misplaced and could be snapped by a slight drag to gain the right position.

Non-maximized window:
With a non-maximized window it was most often possible to hide the main menu by a subsequent click. It was not possible anymore when bug 1623974 landed.

Fullscreen window:
Drag the maximized window slightly to snap into the correct position. Press F11. If the window is misplaced, try to hover the invisible tab bar, wait a bit for it to open, right click on it and uncheck "hide toolbars", then drag the window slightly so it snaps into the correct fullscreen position. Then click on the main menu button and try to close it with a subsequent click.
Already affected before bug 1623974.
2020-03-20 bad
2020-02-20 bad
2020-01-20 bad
2019-12-20 good
MOZ_ENABLE_WAYLAND=1 mozregression --good 2019-12-20 --bad 2020-01-20 --pref gfx.webrender.all:true widget.wayland_vsync.enabled:false

15:55.24 INFO: Last good revision: 39f34852842e1d58860156cba9bc05e8a133a27a
15:55.24 INFO: First bad revision: f4bba00d59cef29deb745e608689e58d45b80394
15:55.24 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=39f34852842e1d58860156cba9bc05e8a133a27a&tochange=f4bba00d59cef29deb745e608689e58d45b80394

f4bba00d59cef29deb745e608689e58d45b80394 Jan Horak — Bug 1607404 - Update mBounds when gdk_window_move_to_rect returns different position/size; r=stransky

Comment 1

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

Jan, looks like a regression from your patch.

Comment 3

[Robert Mader [:rmader]]

So the offending call appears to be the NotifyWindowMoved at https://searchfox.org/mozilla-central/source/widget/gtk/nsWindow.cpp#1548 - which appears to have lots of consequences :(

Edit: and the reason why we continuously get into here is because the position of the window constantly toggles between 425,99 and 425,100 - looks almost like a off-by-one error somewhere :P
Maybe some rounding error somewhere?

Comment 4

[Robert Mader [:rmader]]

Attachment: Bug 1645695 - Only update popup allocation when visible. r?stransky,jhorak

In NativeMoveResizeWaylandPopupCB we update the allocation of popups
and many menus like the hamburger menu get moved to another position here.

When we close the menu, the allocation gets reset in
nsView::ResetWidgetBounds, making us call NativeMoveResizeWaylandPopup,
triggering NativeMoveResizeWaylandPopupCB to set the desired size again,
making us trigger NativeMoveResizeWaylandPopup again - confusing our
calculation of isWidgetVisible, making us show the menu again when it
should be hidden.

To avoid this, don't connect NativeMoveResizeWaylandPopupCB when the
widget is not visible.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c0b1e3fab14
Only update popup allocation when visible. r=stransky

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/0c0b1e3fab14

Comment 7

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

We need to backout this patch as it causes a significant regression in popup handling (Bug 1687979).
Can you please revert the patch here?
Thanks.

Comment 8

[Dorel Luca [:dluca]]

Backed out changeset 0c0b1e3fab14 (bug 1645695) for regressing Bug 1687979. a=backout

Backout:
https://hg.mozilla.org/mozilla-central/rev/94786b54b7f3dfdd597f0f89e049e531311ccaed

Comment 9

[Robert Mader [:rmader]]

Hm alright, will have another look into this next cycle.

Comment 10

[Robert Mader [:rmader]]

Digged a bit deeper here, some more info:

• I could track the reopening to XULPopupElement::OpenPopup() - however, there we leave c-land, so this apparently gets called from js
• it appears to me as if this is related to the eXULPopupHidden mouse event dispatched in nsXULPopupManager::HidePopupCallback()
• somehow all of this is trigged by NotifyWindowMoved() in NativeMoveResizeWaylandPopupCB() - disabling that code part solves the issue and I've not yet seen any draw-back from it - however it's probably not the correct fix.

Comment 11

[Robert Mader [:rmader]]

Looks like this was fixes by the UI rework \o/

Keeping it open a bit longer, but for me it doesn't reproduce any more on nightly.

Comment 12

[Kestrel]

It was fixed by Bug 1696253 but then broken by Bug 1701920 which was reverted by Bug 1706415, so currently it is fixed but it should be expected to break again. In the broken state, the menu flickers when opened which seems related to the issue.

Comment 13

[Viktor Jägersküpper]

With the current Nightly, I often get

 Gdk-CRITICAL **: gdk_wayland_window_remove_frame_callback_surface: assertion 'surface != NULL' failed

when I click on the button to open the menu. I don't see this with the 88 release. Would it help if I use mozregression to find out what caused this?

Comment 14

[Viktor Jägersküpper]

(In reply to Viktor Jägersküpper from comment #13)

With the current Nightly, I often get

 Gdk-CRITICAL **: gdk_wayland_window_remove_frame_callback_surface: assertion 'surface != NULL' failed

when I click on the button to open the menu.

Addressed by bug 1710176.

Comment 15

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

Can you please test latest nightly under Wayland? A new popup handling code landed there.
Thanks.

Comment 16

[joshas]

Tested firefox-91.0a1.en-US.linux-x86_64 on Fedora 33 with Wayland. Menu button works as expected: click opens menu, another click closes menu. No more blinking on subsequent clicks.

Comment 17

[Emmanuel M. González]

This start happens again on Fedora 34 Workstation. Version: firefox-90.0-3.fc34.x86_64 / firefox-92.0a1.es-AR.linux-x86_64

Maybe I have to say this in other ticket but exactly the same happens to every other extension/tool that have a menu and is in the left of the address bar .

Comment 18

[Kestrel]

Yes, I can confirm it is still reproducible when the popup (hamburger menu and extensions) does not fit in the screen and needs to reposition itself.

Comment 20

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

Can you please test latest nightly under Wayland?
https://fedoraproject.org/wiki/How_to_debug_Firefox_problems?rd=Bug_info_Firefox#Testing_Mozilla_binaries
Thanks.

Comment 21

[Kestrel]

I can still reproduce this on Nightly 94.0a1 (2021-09-12) with Wayland on Ubuntu 21.04 by dragging the window close to the bottom edge of the screen so the hamburger menu has to reposition when opened.

Comment 22

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

Works for me in stock Firefox 93, can you re-test please?

Comment 23

[Kestrel]

Attachment: popup-sequent-click.webm

Still reproducible for me on 93 and latest Nightly 95 on Ubuntu 21.04.

Comment 24

[Kestrel]

Works for me on latest Nightly 106.0a1 on Ubuntu 22.04.

Fix window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=bb5b8cfe16b0ed24a018a571f1133522acb5680f&tochange=2f18062042c847788ddda289f5d09186450770d9

Fixed by Bug 1784876.