1701637 - [macOS] Bookmarks toolbar button submenu item hover states are low-contrast

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P2)

Description

[Harry Twyford]

Attachment: Screen Shot 2021-03-29 at 8.09.28 AM.png

The submenus in the bookmarks toolbar button are styled like native menus, at least on macOS. We appear to apply the Secondary Hover bgcolor to them on hover. This is very low-contrast. We might consider using a blue/system Highlight hover bgcolor for the items in these submenus.

Comment 1

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

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

Comment 2

[Harry Twyford]

Attachment: Screen Shot 2021-03-29 at 1.23.31 PM.png

This is not an issue in Dark mode, where we style the submenu background.

Comment 3

[Romain Testard [:RT]]

Martin, is this a platform issue?

Comment 4

[Martin Balfanz [:mbalfanz]]

Attachment: Screenshot 2021-03-30 at 18.48.52.png

Those are not native menus, so I don't think it's a platform issue.

Also FYI, on 10.15 the contrast appears fine.

Comment 5

[:Gijs (he/him)]

Harry, can you confirm STR here? Are you still seeing this, and if so, does this only happen on big sur?

Comment 6

[Harry Twyford]

Yes, I'm still seeing this on the latest Nightly.

STR:

1. In the Customize screen, add the Bookmarks Menu button to the toolbar.
2. Click the bookmarks toolbar button to open the bookmarks panel.
3. Hover a folder in the panel to open its submenu.
4. Hover a bookmark in the submenu.

I'm not sure about macOS versions before Big Sur. I don't have any machines running macOS < 11. Martin's comment 4 suggests it's not an issue there.

Comment 7

[Martin Balfanz [:mbalfanz]]

Attachment: default-vs-light.png

I tried this a little bit more, and I'm also able to reproduce this on 10.15.

To reproduce the issue, it's important to pick the default theme. See the comparison between default and light theme in the attachment. (Also note the issue with the corners in the default theme, it's not really round.)

Comment 8

[:Gijs (he/him)]

Thanks for the clarifications.

This appears to be because the first menupopup gets the places-popup-arrow binding which gets styled like an arrow panel (except we removed the arrows for proton...) and the submenus all get places-popup. This is the same root cause as bug 1702945.

We can probably fix the macOS case with yet more custom CSS, but it's not a very sustainable setup, the whole "Let's take a menupopup and make it look like a panel but behave like a menu..." thing is just a recipe for brokenness. :-\

Comment 9

[:Gijs (he/him)]

Attachment: Bug 1701637 - fix bookmarks menu button submenu styling in proton, r?mconley

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29a1f81830bb
fix bookmarks menu button submenu styling in proton, r=mconley

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/29a1f81830bb

Comment 12

[:Gijs (he/him)]

Romain, do you want us to uplift this? It's a pretty straightforward CSS change, marked p2a.

Comment 13

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #12)

Romain, do you want us to uplift this? It's a pretty straightforward CSS change, marked p2a.

Passing this to Shilpa.

Comment 14

[Shilpa Mohanty]

Request for Uplift since this is a small change. I'l remove the wontfix flag.

Comment 15

[:Gijs (he/him)]

Comment on attachment 9220148 [details]
Bug 1701637 - fix bookmarks menu button submenu styling in proton, r?mconley

Beta/Release Uplift Approval Request

• User impact if declined: Bookmarks menu button subfolder items are not readable on some versions of macOS
• 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. Note that the fix affects Windows and Linux too, so it'd be good to check that the menus look OK there too.
• List of other uplifts needed: n/a
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): CSS-only changes that only affect the bookmarks menu button
• String changes made/needed: Nope

Comment 16

[(Out Sep 17-23) Ryan VanderMeulen]

Comment on attachment 9220148 [details]
Bug 1701637 - fix bookmarks menu button submenu styling in proton, r?mconley

Approved for 89.0b12.

Comment 17

[(Out Sep 17-23) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-beta/rev/321273b898f6

Comment 18

[Oana Botisan, Desktop QA]

Verified the fix on Firefox 89.0b13 and Nightly 90.0a1 using macOS 11, Windows 10 and Ubuntu 18.04. The issue is not reproducing anymore and the fix didn't cauze any additional issues.