1852153 - A dropdown menu flickered when it overlapped the address bar and the cursor moved over it in Firefox on Wayland

Status: VERIFIED FIXED

(Core :: Widget: Gtk, defect)

Description

[Matt Fagnani]

Attachment: firefox-119.0a1-2023-09-07-menu-flickering-1.mp4

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

Steps to reproduce:

I started Firefox 119.0a1 20230907092106 on Wayland with WebRender compositing in Plasma 5.27.7 in a Fedora 39 KDE Plasma installation. I had previously enabled the menu bar. I loaded bugzilla.mozilla.org. I logged into my account. I selected New Bug > Other Products > Release Engineering. I entered a summary and clicked on Find similar issues and My issue is not listed. I clicked on the Component dropdown menu. I moved the cursor up and down over the Component dropdown menu.

Actual results:

The Component dropdown menu was shown upward and overlapped with the address bar. The Component dropdown menu flickered when I moved the cursor over it. I'm attaching a recording of this problem. The menu bar must be enabled for this flickering to happen. If the menu bar wasn't enabled, the Component dropdown menu was shown downward and didn't flicker.

Using mozregression, I found that the first bad revision was for Bug 1828413 - Use more non-native rendering of menus.

23:04.72 INFO: Narrowed integration regression window from [7a4c240d, df18ff71] (4 builds) to [1f216436, df18ff71] (2 builds) (~1 steps left)
23:04.72 INFO: No more integration revisions, bisection finished.
23:04.72 INFO: Last good revision: 1f216436fe2b858a7b7e9a22517e60ac222c40ed
23:04.72 INFO: First bad revision: df18ff71a63d4d18e7cf9d6d2a0b0f0662076264
23:04.72 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=1f216436fe2b858a7b7e9a22517e60ac222c40ed&tochange=df18ff71a63d4d18e7cf9d6d2a0b0f0662076264

Expected results:

The Component dropdown menu shouldn't have flickered.

Comment 1

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

:emilio, since you are the author of the regressor, bug 1828413, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Just for future reference, in order for me at least to see the form I need to go directly here.

I don't think this is really a regression from bug 1828413. That bug just changed the styles enough so that the height is different. Same reason why the menubar seems to introduce this bug. It just makes that particular select element high enough...

That said, I can repro if the window is maximized and I size my display to 1707x960px. So will investigate.

Curious, can you repro if the window is not maximized? I can't there (the element is scrollable inside the content bounds, as it should).

Thanks!

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Didn't mean to cancel ni?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced test-case that reproduces the issue for me.

I think our popup positioning code is getting confused because:

• We try to show it to the bottom, but there's not enough space.
• So the compositor gives us the flipped size.
• But there's not enough space there either to achieve the other constraint (the popup not overflowing into the browser UI).

So it's a weird situation but something we should address.

Comment 5

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

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

Comment 6

[Matt Fagnani]

When I make Firefox vertically smaller, then the Component menu doesn't overlap the address bar and doesn't flicker. The menu shows a scroll bar in that case. When I make Firefox smaller just horizontally, then the Component menu still overlaps the address bar and flickers in the same way as I reported. Thanks.

Comment 7

[Matt Fagnani]

Attachment: Dropdown menu flickering on archive.org

I saw a dropdown menu called Collection Search flickering at https://web.archive.org/web/20230000000000*/firefox which slightly overlapped the address bar and had a scroll bar. The menu had a height which was about the same as the main part of the window. I'm attaching a recording of the flickering menu which might be another example of this problem.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Yeah, same issue.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1852153 - Don't log in GetTopLevelWindow. r=stransky

Now we call this more often it becomes very noisy.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1852153 - Let GTK know about shadow sizes so that move_to_rect agrees with our constraining code. r=stransky

This makes GTK and our sizing code agree in allowing to position popups
slightly off-screen to account for shadows.

This gives us the correct move-to-rect positions. However, it seems like
this breaks the popups intermittently.

Depends on D187799

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Martin, can you poke at comment 10 and see if there's anything obvious missing? The popups are there and can be hovered etc (see incoming test-case), they're just not visible?

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Attachment: Test-case for comment 10

The 0 margin makes move-to-rect kick in because the popup should be directly under the element and slightly off-screen.

Comment 13

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

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

Martin, can you poke at comment 10 and see if there's anything obvious missing? The popups are there and can be hovered etc (see incoming test-case), they're just not visible?

Wayland compositor may decide to not show popup if it's not adjacent to parent window. In such case we're not notified about it - move-to-rect is no-op in such case. There's a difference between compositors, Mutter doesn't show anything, KDE places the popup on weird location.

Firefox perspective the popup is show so we show tooltips etc as the window is visible. MOZ_LOG="WaylandPopup:5" may show it. I think disabled move-to-rect may fix it.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Well but they are adjacent right? Ok let me take another look when time allows

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15116d1c85c2
Don't log in GetTopLevelWindow. r=stransky

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1852153 - Don't use move-to-rect if layout has constrained popup. r=stransky

This is an edge case where layout thinks the popup doesn't need
flipping, but the compositor does.

In an ideal world we'd convince the compositor it doesn't by providing
our margins, but that doesn't seem easily doable.

So check whether there was a constraint rect that we tried to honor
instead, rather than whether we actually had to flip or what not.

Comment 17

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/15116d1c85c2

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1a06649dfef6
Don't use move-to-rect if layout has constrained popup. r=stransky

Comment 19

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/1a06649dfef6

Comment 20

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

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-firefox118 to wontfix.

For more information, please visit BugBot documentation.

Comment 21

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9352317 [details]
Bug 1852153 - Don't use move-to-rect if layout has constrained popup. r=stransky

Beta/Release Uplift Approval Request

• User impact if declined: comment 0
• 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: comment 0 or comment 4
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Relatively simple fix.
• String changes made/needed: None
• Is Android affected?: No

Comment 22

[Pascal Chevrel:pascalc]

Comment on attachment 9352317 [details]
Bug 1852153 - Don't use move-to-rect if layout has constrained popup. r=stransky

Approved for landing on mozilla-beta before the merge, will be in the 118.0 release candidate, thanks.

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/7b784fc3c494

Comment 24

[Anca Soncutean, Desktop QA]

I couldn't replicate the problem on my Ubuntu 22 (Wayland) with Nightly 119.0a1 (2023-09-07). I don't have access to a Fedora system. I followed the instructions detailed in the bug report and use the provided test cases. I'm unable to initiate the overlapping state, regarding the screen resolution or the Firefox dimension. The only flickering I observed was when the dropdown expanded beyond the browser's width and this flicker isn't present in the most recent Nightly version, but it seams independent from the problem targeted here.
Matt Fagnani, can you please, if your time permits, verify the fix for this issue on the latest RC build? Thank you!

Comment 25

[Matt Fagnani]

The flickering I reported didn't happen on the pages I reported or the test cases with 119.0a1 (2023-9-19) on Wayland or 118.0 RC-build1 20230918143747. The dropdown menus went downward so they didn't overlap the address bar. Thanks.

Comment 26

[Anca Soncutean, Desktop QA]

(In reply to Matt Fagnani from comment #25)

The flickering I reported didn't happen on the pages I reported or the test cases with 119.0a1 (2023-9-19) on Wayland or 118.0 RC-build1 20230918143747. The dropdown menus went downward so they didn't overlap the address bar. Thanks.

Thank you for your prompt reply!
Based on comment 25, I will mark this issue as Verified fixed.