1871515 - [Wayland] Popup is not scrollable on HiDPI

Status: RESOLVED FIXED

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

Description

[Lyndon]

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

Steps to reproduce:

Using Firefox 121.0-1 on Debian Sid.

The vertical tab list (accessed via the 'down arrow' in the tab bar, to
the right of the new tab '+' button) is now no longer working correctly
for Hi-DPI as of version 121.0-1.

I have many windows with many tabs. Previously with version 120 I had
no problem scrolling the entire tab list. Now, depending upon which tab
I have selected, I cannot scroll a certain number of tabs at the top
and/or bottom of the list into view.

Notably when trying to scroll to the very top/bottom I see the
scrollbar going out of view beyond the limits of the screen height.

I have a Hi-DPI screen set to 200% scaling. Experimenting, if I switch
to 100% scaling then the problem goes away, being able to scroll the
full list as previously (with the list drawn with 100% screen height
regardless of whether the window is maximized or not).

So it seems that a small regression was introduced.

As requested in the original Debian bug report (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1059241) I tried running with MOZ_ENABLE_WAYLAND=0 and the problem does not exist when doing so.

Comment 1

[Lyndon]

Using Gnome btw.

Comment 2

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

Can you attach screencast of the issue?
https://fedoraproject.org/wiki/How_to_debug_Firefox_problems#Collect_information_for_a_bug_report
Thanks.

Comment 3

[Lyndon]

I'm reluctant to make a screencast of my actual loaded tabs for privacy reasons, but I'm trying to see what I can do about recreating the issue in a new window.

Worth mentioning - in a new window with only a few new-tabs added, the tab droplist for this window is displayed just fine, it's in the correct location with the correct height, not taking up 100% screen height. This is also the case for some of the other windows I have open that contain only a small number of tabs, so the problem seems to only affect windows with large numbers of tabs.

Comment 4

[Lyndon]

Sorry for the delay, I've been having some trouble with an error when trying to create screencasts but I've finally managed to resolve that by installing wireplumber.

Comment 5

[Lyndon]

Attachment: Screencast 1

Okay, first screencast attached. It starts having pre-opened a new window with a single tab opened to bugzilla.

In the first 12 seconds you see me add an additional 19 new tabs (via ctrl+t, for 20 total), load bugzilla in the last (20th tab), and then check the tab droplist, which at this point is fine.

From 0:13 to 0:27 I then add an additional 20 tabs (for 40 total) and load bugzilla in the last (40th). From 0:28 to 0:31 I recheck the tab list again. At this point the list is shown at full screen height with only the first 25 tabs visible along with the search tabs entry. When trying to scroll the list nothing happens (no movement); presumably this is because its height would fit fully on the screen if I had 100% DPI scaling rather than 200%.

From 0:32 to 0:42 I add an additional 20 tabs (for 60 total) and load bugzilla in the last (60th). From 0:43 to end I recheck the tab list again. At this point the list is scrollable, but as you can tell from seeing the 'search tabs' entry at the beginning of the list and the intermittent bugzilla tabs (1, 20, 40, 60), scrolling up and down we can only see tabs 1-33, tabs 34-60 are unreachable.

Oddly I'm struggling to reproduce the behaviour where it cuts off the top of the list rather than the bottom. I'd previously observed with one of my other windows that if the current active tab is the first one then the bottom of the list is cut off, whilst if the last tab is selected then the front of the list would be cut off instead. But now with both this new test window and the others I'm finding it impossible to reliably recreate the condition where the front of the list is cut off, it's just always cutting off the end.

Comment 6

[Lyndon]

Attachment: Screencast 2

Here's screencast number 2. For this one I've repeated the same procedure except with DPI scaling change from 200% to 100%, the window is not full screen, and I open 100 tabs total rather than 60. So again I'm adding batched 20 new tabs with the last being opened to bugzilla.

As you can see the full list is scrollable in this case even at 100 tabs.

Comment 7

[Lyndon]

Typo, meant to say "I'm adding batches of 20", so hard to see at 100% scaling.

Comment 8

[Lyndon]

Attachment: Screencast 3

And here's screencast 3 in which I repeated what I did for screencast 1 but this time running firefox with MOZ_ENABLE_WAYLAND=0. As you can see, no issues scrolling the tab list.

Comment 9

[Lyndon]

Presumably under wayland ff version 121 is just failing to adjust for DPI scaling in a list height calculation.

Comment 10

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

I see. Do you use fractional scale. i.e. do you have 125% 150% ... scale options at Display config?
Also can you try to flip widget.wayland.use-move-to-rect at about:config, restart browser and try again?

Comment 11

[Lyndon]

No, gnome settings only offer choices of 100%, 200% and 300%.

Flipping widget.wayland.use-move-to-rect, the problem still exists. The setting was originally set to true. I opened about:config in a new tab of a window with many tabs. I flipped the setting to false, restarted FF, and checked the tab list for that same window. The list was correctly drawn just beneath the button that activates it, but I could not scroll the full list. I flipped the setting back to true, restarted, checked again, this time list was drawn on top of the button that activates it, and I also could not scroll the full list.

Comment 12

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

Yes, I see that on 200% too.

Comment 13

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

Looks like if 'Wayland constraint height' is applied we're missing the scrollbars:
https://searchfox.org/mozilla-central/rev/109bb25545f0d2df31954dc0a9afbf30d900b6bb/layout/xul/nsMenuPopupFrame.cpp#1580

Emilio, maybe another reflow is requested then? But I wonder why it affects Wayland & scaled displays only. Maybe reflow takes unscaled values?
Thanks.

Comment 14

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

So yes, if I open too many tabs so it overflow unscaled popup size, the scrollbar is here. So we reflow the popup with unscaled coords.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

This is because we don't get the right screen height, I think this is just https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/6723. Does it work with gnome fractional scaling enabled?

I see a related issue with monitors with different screen sizes (e.g., if you have a screen with 3x scale and one with 2x scale). That is because Element::GetScreen uses ScreenForRect, which can't really work on Wayland.

So we should probably switch Element::GetScreen to GetNearestWidget()->GetWidgetScreen() or so to fix that.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1871515 - Use GetWidgetScreen to get Element.screen. r=stransky

So that we get the right screen on Wayland, where ScreenForRect is not
really reliable.

This makes mismatched screen sizes and resolutions work better. In
particular, this matches what window.screen returns.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1871515 - Let nsMenuPopupFrame constrain the panelview, and remove _calculateMaxHeight. r=Gijs,dao

This removes some complex code, and improves the behavior on platforms
where screen sizes might not be 100% reliable. To be landed after the
soft-freeze of course.

Comment 18

[Phabricator Automation]

Comment on attachment 9391437 [details]
Bug 1871515 - Use GetWidgetScreen to get Element.screen. r=stransky

Revision D204753 was moved to bug 1885521. Setting attachment 9391437 [details] to obsolete.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53d726685bbb
Let nsMenuPopupFrame constrain the panelview, and remove _calculateMaxHeight. r=dao,desktop-theme-reviewers

Comment 21

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/53d726685bbb