The current banner looks roughly like this on the dark theme (emulated the banner using the browser toolbox). It would be nice if it were lighter similar to bug 1452674.

Amy, would it be possible to get a specification for the colors ?

Looks like there's also a FxA banner that uses a similar color.

I've attached a spec to update the colours for attention notifications.

Thanks Amy! This bug can be solved by adding new rules for `:root[lwt-popup-text]` in: https://searchfox.org/mozilla-central/source/browser/themes/shared/customizableui/panelUI.inc.css#454,594-595 using the colors in attachment 8968233 [details]

Comment on attachment 8975884 [details] Bug 1454231 - Improved dark theme colors of popup warnings. https://reviewboard.mozilla.org/r/244080/#review251558 Thanks for the patch and sorry for the delay in getting you some feedback. I've got some questions below about your patch. ::: browser/themes/shared/customizableui/panelUI.inc.css:112 (Diff revision 1) > - background: #FFBF00 url(chrome://browser/skin/update-badge-failed.svg) no-repeat center; > + background: #FFBF00 no-repeat center; > + mask-image: url(chrome://browser/skin/warning-clear.svg); > +} > + > +:root[lwt-popup-brighttext] #PanelUI-menu-button[badge-status="addon-alert"] > .toolbarbutton-badge-stack > .toolbarbutton-badge { > + background: #FFE900 no-repeat center; This only needs to set background-color. ::: browser/themes/shared/customizableui/panelUI.inc.css:466 (Diff revision 1) > height: 16px; > } > > .addon-banner-item::after { > - background: #FFBF00 url(chrome://browser/skin/update-badge-failed.svg) no-repeat center; > + background: #FFBF00 no-repeat center; > + mask-image: url(chrome://browser/skin/warning-clear.svg); Why did you need to introduce a new SVG image? If you just want to remove the color, you can update update-badge-failed.svg to have fill="context-fill" instead of fill="#fff". And then at the places in the CSS where it is referenced you would set these two properties: -moz-context-properties: fill; fill: #fff (or other color of your choice) ::: browser/themes/shared/customizableui/panelUI.inc.css:470 (Diff revision 1) > - background: #FFBF00 url(chrome://browser/skin/update-badge-failed.svg) no-repeat center; > + background: #FFBF00 no-repeat center; > + mask-image: url(chrome://browser/skin/warning-clear.svg); > +} > + > +:root[lwt-popup-brighttext] .addon-banner-item::after { > + background: #FFE900 no-repeat center; This only needs to set background-color. ::: browser/themes/shared/customizableui/panelUI.inc.css:618 (Diff revision 1) > } > > +:root[lwt-popup-brighttext] .addon-banner-item { > + color: @appmenuWarningColorDark@; > + background: @appmenuWarningBackgroundColorDark@; > + border: 0 !important; Why does border:0 need to be set? And why does this need to be !important?

Comment on attachment 8975884 [details] Bug 1454231 - Improved dark theme colors of popup warnings. https://reviewboard.mozilla.org/r/244080/#review251558 > Why does border:0 need to be set? And why does this need to be !important? :amylee asked for there to be no borders on dark themed warnings. !important is set to override the preexisting border-top !important on line 440.

Comment on attachment 8975884 [details] Bug 1454231 - Improved dark theme colors of popup warnings. https://reviewboard.mozilla.org/r/244080/#review251856 ::: browser/themes/shared/customizableui/panelUI.inc.css:108 (Diff revision 2) > } > > #PanelUI-menu-button[badge-status="addon-alert"] > .toolbarbutton-badge-stack > .toolbarbutton-badge { > height: 13px; > - background: #FFBF00 url(chrome://browser/skin/update-badge-failed.svg) no-repeat center; > + background: #FFBF00 no-repeat center; > + mask-image: url(chrome://browser/skin/warning-clear.svg); Please don't use mask-image. You can leave the background as it was and use context-fill (-moz-context-properties: fill; fill: ...) to change the icon color

Comment on attachment 8975884 [details] Bug 1454231 - Improved dark theme colors of popup warnings. https://reviewboard.mozilla.org/r/244080/#review251952 ::: browser/themes/shared/customizableui/panelUI.inc.css:108 (Diff revision 2) > } > > #PanelUI-menu-button[badge-status="addon-alert"] > .toolbarbutton-badge-stack > .toolbarbutton-badge { > height: 13px; > - background: #FFBF00 url(chrome://browser/skin/update-badge-failed.svg) no-repeat center; > + background: #FFBF00 no-repeat center; > + mask-image: url(chrome://browser/skin/warning-clear.svg); I was experimenting with setting it like this: ``` .addon-banner-item::after { background: url(chrome://browser/skin/warning-white.svg) no-repeat center; -moz-context-properties: fill; fill: #FFBF00; } :root[lwt-popup-brighttext] .addon-banner-item::after { fill: #FFE900; } ``` but the icon just appears black no matter what. What am I doing wrong? The new design spec calls for just a coloured icon, rather than a white icon against a colored background, so setting background-color doesn't solve the issue.

Comment on attachment 8975884 [details] Bug 1454231 - Improved dark theme colors of popup warnings. https://reviewboard.mozilla.org/r/244080/#review252234 You can use `hg histedit` to roll up your commits.

Comment on attachment 8979568 [details] Bug 1454231 - Improved dark theme colors of popup warnings. https://reviewboard.mozilla.org/r/245700/#review252236 ::: browser/base/content/browser-addons.js:508 (Diff revision 1) > > while (container.firstChild) { > container.firstChild.remove(); > } > > + this._createAddonButton("test", null, null); This should be deleted. ::: browser/themes/shared/customizableui/panelUI.inc.css:108 (Diff revision 1) > } > > #PanelUI-menu-button[badge-status="addon-alert"] > .toolbarbutton-badge-stack > .toolbarbutton-badge { > height: 13px; > background: #FFBF00 no-repeat center; > - mask-image: url(chrome://browser/skin/warning-clear.svg); > + mask-image: url(chrome://browser/skin/warning-white.svg); Can you please go back to using the previous image and not introduce another one? ::: browser/themes/shared/customizableui/panelUI.inc.css:470 (Diff revision 1) > > .addon-banner-item::after { > background: #FFBF00 no-repeat center; > - mask-image: url(chrome://browser/skin/warning-clear.svg); > + mask-image: url(chrome://browser/skin/warning-white.svg); > + -moz-context-properties: fill; > + fill: #fff semi-colon needed at the end and no trailing white space. ::: browser/themes/shared/notification-icons.inc.css:158 (Diff revision 1) > } > > #webRTC-previewWarning { > background: rgba(255, 217, 99, .8) url("chrome://browser/skin/warning-white.svg") no-repeat .75em .75em; > + -moz-context-properties: fill; > + fill: #fff semi-colon needed at the end. ::: browser/themes/shared/warning-white.svg:5 (Diff revision 1) > <!-- This Source Code Form is subject to the terms of the Mozilla Public > - License, v. 2.0. If a copy of the MPL was not distributed with this > - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16"> > - <path fill="#fff" stroke="#000" stroke-opacity="0.3" d="M15.4,12.9 9.46,1.41 C9.12,0.756 8.59,0.381 8,0.381 7.41,0.381 6.88,0.756 6.54,1.41 L0.642,12.9 c-0.331,0.6 -0.348,1.3 -0.05,1.9 0.299,0.5 0.854,0.8 1.534,0.8 H13.9 c0.6,0 1.2,-0.3 1.5,-0.8 0.3,-0.6 0.3,-1.3 0,-1.9z M8.83,5.07 8.65,10.5 H7.34 L7.15,5.07 H8.83z M8,13.7 c-0.55,0 -0.99,-0.5 -0.99,-1 0,-0.6 0.44,-1 0.99,-1 0.56,0 0.99,0.4 0.99,1 0,0.5 -0.43,1 -0.99,1z"/> > + <path fill="#context-fill" stroke-opacity="0.3" d="M15.4,12.9 9.46,1.41 C9.12,0.756 8.59,0.381 8,0.381 7.41,0.381 6.88,0.756 6.54,1.41 L0.642,12.9 c-0.331,0.6 -0.348,1.3 -0.05,1.9 0.299,0.5 0.854,0.8 1.534,0.8 H13.9 c0.6,0 1.2,-0.3 1.5,-0.8 0.3,-0.6 0.3,-1.3 0,-1.9z M8.83,5.07 8.65,10.5 H7.34 L7.15,5.07 H8.83z M8,13.7 c-0.55,0 -0.99,-0.5 -0.99,-1 0,-0.6 0.44,-1 0.99,-1 0.56,0 0.99,0.4 0.99,1 0,0.5 -0.43,1 -0.99,1z"/> This should just be fill="context-fill", not fill="#context-fill"

Comment on attachment 8979568 [details] Bug 1454231 - Improved dark theme colors of popup warnings. https://reviewboard.mozilla.org/r/245700/#review253000 ::: browser/themes/shared/customizableui/panelUI.inc.css:620 (Diff revision 2) > } > > +:root[lwt-popup-brighttext] .addon-banner-item { > + color: @appmenuWarningColorDark@; > + background: @appmenuWarningBackgroundColorDark@; > + border: 0 !important; Please include a comment with the reason why you need to use !important here. ::: browser/themes/shared/notification-icons.inc.css:156 (Diff revision 2) > background: rgba(255, 217, 99, .8) url("chrome://browser/skin/warning-white.svg") no-repeat .75em .75em; > + -moz-context-properties: fill; > + fill: #fff; Can we remove warning-white.svg and use warning.svg here, and update warning.svg to use context-fill and context-stroke? The name warning-white.svg doesn't make sense anymore anyways since the file doesn't use the color white in there now.

I should say that this is looking good and on the right path. I think with the above requested changes made this will be ready to land :)

Comment on attachment 8979568 [details] Bug 1454231 - Improved dark theme colors of popup warnings. https://reviewboard.mozilla.org/r/245700/#review254756 ::: browser/themes/shared/warning.svg:5 (Diff revision 3) > <!-- This Source Code Form is subject to the terms of the Mozilla Public > - License, v. 2.0. If a copy of the MPL was not distributed with this > - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > -<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"> > - <path fill="#ffbf00" d="M14.8,12.5L9.3,1.9C9,1.3,8.5,1,8,1C7.5,1,7,1.3,6.7,1.9L1.2,12.5c-0.3,0.6-0.3,1.2,0,1.7C1.5,14.7,2,15,2.6,15h10.8 c0.6,0,1.1-0.3,1.4-0.8C15.1,13.7,15.1,13.1,14.8,12.5z"/> > +<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16"> > + <path fill="context-fill" stroke="context-stroke" stroke-opacity="0.3" d="M15.4,12.9 9.46,1.41 C9.12,0.756 8.59,0.381 8,0.381 7.41,0.381 6.88,0.756 6.54,1.41 L0.642,12.9 c-0.331,0.6 -0.348,1.3 -0.05,1.9 0.299,0.5 0.854,0.8 1.534,0.8 H13.9 c0.6,0 1.2,-0.3 1.5,-0.8 0.3,-0.6 0.3,-1.3 0,-1.9z M8.83,5.07 8.65,10.5 H7.34 L7.15,5.07 H8.83z M8,13.7 c-0.55,0 -0.99,-0.5 -0.99,-1 0,-0.6 0.44,-1 0.99,-1 0.56,0 0.99,0.4 0.99,1 0,0.5 -0.43,1 -0.99,1z"/> You can use fill="context-fill #FFBF00" to specify a default color to use when fill isn't specified, to avoid having to specify the same fill all the time. Same for context-stroke.

Comment on attachment 8979568 [details] Bug 1454231 - Improved dark theme colors of popup warnings. https://reviewboard.mozilla.org/r/245700/#review255176 ::: browser/themes/shared/customizableui/panelUI.inc.css:30 (Diff revision 4) > +%define appmenuWarningBackgroundColorDark hsla(55, 100%, 50%, 0.1) > +%define appmenuWarningBackgroundColorHoverDark hsla(55, 100%, 50%, 0.15) > +%define appmenuWarningBackgroundColorActiveDark hsla(55, 100%, 50%, 0.2) nit, no spaces after commas in color functions and drop the preceding 0 in front of decimal points. ::: browser/themes/shared/customizableui/panelUI.inc.css:30 (Diff revision 4) > +%define appmenuWarningBackgroundColorDark hsla(55, 100%, 50%, 0.1) > +%define appmenuWarningBackgroundColorHoverDark hsla(55, 100%, 50%, 0.15) > +%define appmenuWarningBackgroundColorActiveDark hsla(55, 100%, 50%, 0.2) > +%define appmenuWarningColorDark #F9F9FA Please replace 'Dark' with 'BrightText' so it is consistent with the other usages brighttext. ::: browser/themes/shared/notification-icons.inc.css:156 (Diff revision 4) > min-width: 300px; > max-height: 200px; > } > > #webRTC-previewWarning { > - background: rgba(255, 217, 99, .8) url("chrome://browser/skin/warning-white.svg") no-repeat .75em .75em; > + background: rgba(255, 217, 99, .8) url("chrome://browser/skin/warning.svg") no-repeat .75em .75em; nit, no spaces after comma in color functions.

(In reply to Tim Nguyen :ntim from comment #18) > You can use fill="context-fill #FFBF00" to specify a default color to use > when fill isn't specified, to avoid having to specify the same fill all the > time. I didn't know this before, thanks for sharing!

Comment on attachment 8979568 [details] Bug 1454231 - Improved dark theme colors of popup warnings. https://reviewboard.mozilla.org/r/245700/#review255208 ::: browser/themes/shared/downloads/downloads.inc.css:146 (Diff revision 4) > + -moz-context-properties: fill, stroke; > + fill: #FFBF00; > + stroke: #fff; This is the default fill/stroke, so you don't need to respecify it.

Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/autoland/rev/0beab09f8058 Improved dark theme colors of popup warnings. r=jaws