1763910 - Main menu items can be cut off, if the localized "Zoom" text is long-ish and when there's a scrollbar

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect, P3)

Description

[Itiel]

STR:

1. Be on a certain monitor configuration (small monitor size / high DPI etc) that would make the main menu scrollable
2. Have a localized version of Nightly that the "Zoom" text on the main menu would be a bit long (or, alternatively, replace the value="Zoom" attribute on the Zoom's label to value="מרחק מתצוגה" (Hebrew), or value="ზომის ცვლილება" (Georgian))
3. Open the main menu

AR:
Menu items are readable and not cut off.

ER:
In the Hebrew case, the menu items are still visible but hovering them makes it obvious something is cutoff from the side.
In the extreme case in Georgian, the shortcuts are almost emtirely cut off.

Comment 1

[Itiel]

Attachment: Screenshot

Comment 2

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

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

Comment 3

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

:sfoster, since you are the author of the regressor, bug 1762148, could you take a look?
For more information, please visit auto_nag documentation.

Comment 4

[Sam Foster [:sfoster] (he/him)]

Thanks, I'll look into this.

Comment 5

[Sam Foster [:sfoster] (he/him)]

This is because the .toolbaritem-combined-buttons:is([cui-areatype="menu-panel"], [overflowedItem=true]) selector I'm using in panelUI-shared.css doesnt match these items when they are in the main menu. There's no cui-areatype attribute on the items in this menu. So, I'm not sure if the best solution is to add/revert to the panelview .toolbaritem-combined-buttons we were using previously (which remember doesnt match these button when they are in the overflow menu in customize mode.) or something else - maybe ensure mainMenu toolbar items do have a cui-areatype attribute?

Comment 6

[Dão Gottwald [:dao]]

For the purpose of uplifting, let's revert to :is(:not([cui-areatype="toolbar"]), [overflowedItem=true]). But then I still think these attributes are pretty unhelpful for CSS (which I think is their main purpose?), so going forward I think we should either figure out how to wrap the customize mode fake panel in a panelview element so we can use that selector, or otherwise revamp / consolidate cui-areatype in a way that makes sense (maybe fold overflowedItem into it as well). Gijs, do you have any further thoughts?

Comment 7

[:Gijs (he/him)]

(In reply to Dão Gottwald [::dao] from comment #6)

For the purpose of uplifting, let's revert to :is(:not([cui-areatype="toolbar"]), [overflowedItem=true]). But then I still think these attributes are pretty unhelpful for CSS (which I think is their main purpose?), so going forward I think we should either figure out how to wrap the customize mode fake panel in a panelview element so we can use that selector, or otherwise revamp / consolidate cui-areatype in a way that makes sense (maybe fold overflowedItem into it as well). Gijs, do you have any further thoughts?

cui-areatype is/was also used for logic, and is meant to indicate the CustomizableUI area type that something is in (a decision from the australis / fx29 days). This meant that items that overflow from the toolbar into the overflow menu have areatype=toolbar, whereas items that were in the then-customizable main menupanel had areatype=panel. When for Photon we made the main hamburger menu non-customizable, but made the overflow panel customizable, the areatype=panel bit migrated there, which led to the confusing situation where:

1. items in the overflow panel can be either areatype=panel or toolbar, depending on whether they are "pinned" to the overflow panel or not
2. items in the hamburger menu have no areatype attribute at all

I think in the short term, the panelview descendant selector and making those work in customize mode is the sensible option. For future ease of maintenance it probably makes sense to more clearly split out any logic relating to area types on the one hand, and attributes/classes around "is this in a panel" on the other. Perhaps the areatype attribute should be renamed in some way.

Hopefully that all makes sense...

Comment 8

[Sam Foster [:sfoster] (he/him)]

Yeah that makes sense. I've opened bug 1764832 for the panelview follow-up - which seems to be the most sane first step here, once we've got the issue patched on beta.

Comment 9

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1763910 - Use :not([cui-areatype=toolbar]) to ensure the combined toolbaritem buttons pick up the right styles when in the hamburger menu. r?Dao!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/053ec80f1a57
Use :not([cui-areatype=toolbar]) to ensure the combined toolbaritem buttons pick up the right styles when in the hamburger menu. r=dao

Comment 11

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/053ec80f1a57

Comment 12

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 9272524 [details]
Bug 1763910 - Use :not([cui-areatype=toolbar]) to ensure the combined toolbaritem buttons pick up the right styles when in the hamburger menu. r?Dao!

Beta/Release Uplift Approval Request

• User impact if declined: The edge of the main menu with the keyboard shortcuts may be cut off for users on low display resolutions where the main menu scrolls.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0. Step 2 (localized version with longer labels) isn't strictly necessary to reproduce.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Well contained patch, restoring original (v99) styles for the combined buttons.
• String changes made/needed: None

Comment 13

[Anca Soncutean, Desktop QA]

We managed to reproduce this issue with Fx 101.0a1 (2022-04-09) (64-bit) on Windows 10 and Windows 10ARM.
Verified fixed with Fx 101.0a1 (2022-04-19) on the same OSs.

Comment 14

[Dianna Smith [:diannaS]]

Comment on attachment 9272524 [details]
Bug 1763910 - Use :not([cui-areatype=toolbar]) to ensure the combined toolbaritem buttons pick up the right styles when in the hamburger menu. r?Dao!

Approved for 100.0b9

Comment 15

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ebae2e2bdd47

Comment 16

[Anca Soncutean, Desktop QA]

Verified fixed with Fx 100.0b9 (treeherder build) on Windows 10 and Windows 10ARM.