1452836 - Dark theme sub menu in the bookmarks toolbar are illegible (Mac OSX)

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Aaron Benson]

Attachment: Bookmark Toolbar Sub Menu.png

When the dark theme is enabled, sub menus in the Bookmarks menu are improperly styled and are illegible. See attached screenshot.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

This works-for-me on Windows but I can reproduce on Mac.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

This is currently marked as a P1 without an assignee. Jared, is there someone at MSU who can work on this or is there someone else available otherwise?

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1452836 - Apply the arrowpanel variables to submenus of the bookmarks popup.

This is the same approach used in the Windows panelUI.css

Review commit: https://reviewboard.mozilla.org/r/237216/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/237216/

Comment 4

[Tim Nguyen :ntim]

Attachment: Screenshot of patch

The menu arrows look invisible on really dark themes like Breeze Dark: https://addons.mozilla.org/en-US/firefox/addon/breeze-dark-theme/

Comment 5

[Tim Nguyen :ntim]

Comment on attachment 8968527 [details]
Bug 1452836 - Apply the arrowpanel variables to submenus of the bookmarks popup.

https://reviewboard.mozilla.org/r/237216/#review243122

I found two issues:
- menu arrows are not very visible
- this removes the macOS vibrancy effect for the default theme

::: browser/themes/osx/customizableui/panelUI.css:42
(Diff revision 1)
> +#BMB_bookmarksPopup menupopup[placespopup=true] {
> +  -moz-appearance: none;
> +  background: transparent;
> +}

This removes the macOS vibrancy effect on the default theme. Can we target only :-moz-lwtheme ?

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8968527 [details]
Bug 1452836 - Apply the arrowpanel variables to submenus of the bookmarks popup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237216/diff/1-2/

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8968527 [details]
Bug 1452836 - Apply the arrowpanel variables to submenus of the bookmarks popup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237216/diff/2-3/

Comment 8

[Tim Nguyen :ntim]

Comment on attachment 8968527 [details]
Bug 1452836 - Apply the arrowpanel variables to submenus of the bookmarks popup.

https://reviewboard.mozilla.org/r/237216/#review243528

Thanks!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/44ea1dc76ef5
Apply the arrowpanel variables to submenus of the bookmarks popup. r=ntim

Comment 10

[Andrei Ciure[:aciure]]

Backed out changeset 44ea1dc76ef5 (bug 1452836) for browser_parsable_css.js failures in chrome://browser/skin/customizableui/panelUI.css on OS X on a CLOSED TREE, a=backout

push that caused the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=44ea1dc76ef50bc77e84ddb26433baf84cb16d84&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=174410093&filter-searchStr=OS+X+10.10+opt+Mochitests+with+e10s+test-macosx64%2Fopt-mochitest-browser-chrome-e10s-7+M-e10s%28bc7%29

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=174410093&repo=autoland

backout: https://hg.mozilla.org/integration/autoland/rev/c064e452478eba9d8be71974a79c18fe0dc46acc

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/31dc1b39ba36
Apply the arrowpanel variables to submenus of the bookmarks popup. r=ntim

Comment 12

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/31dc1b39ba36

Comment 13

[Gabi Cheta [:Gabi] Desktop QA]

I reproduced the issues on MacOS 10.13.2 build 61.0a1(2018-04-05.
Confirming the fix on 61.0b9 (64-bit) MacOS 10.13.2.