Mike de Boer [:mikedeboer] Reporter
	Description • 7 years ago

See (again) https://docs.google.com/spreadsheets/d/1YScpOVL5WdNDhQY2Nngh4YkK0bOpkfwJvpRjpMSxMWo/edit#gid=0

Constants as defined in https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/themes/theme_properties.h are:
 * COLOR_TOOLBAR_BOTTOM_SEPARATOR
 * COLOR_TOOLBAR_TOP_SEPARATOR
 * COLOR_TOOLBAR_VERTICAL_SEPARATOR

It's important here to first implement the properties in the order and with names that fit with Firefox well and try to match these up with the Google equivalents above.

	Jared Wein [:jaws] (please needinfo? me)
	Comment 4 • 7 years ago
mozreview-review

Comment on attachment 8923638 [details]
Bug 1347190 - Add support for toolbar_top/bottom/vertical_separator in the theming API.

https://reviewboard.mozilla.org/r/194764/#review201522

::: toolkit/modules/LightweightThemeConsumer.jsm:23
(Diff revision 3)
>    ["--lwt-background-tiling", "backgroundsTiling"],
>    ["--toolbar-bgcolor", "toolbarColor"],
>    ["--toolbar-color", "toolbar_text"],
>    ["--url-and-searchbar-background-color", "toolbar_field"],
> -  ["--url-and-searchbar-color", "toolbar_field_text"]
> +  ["--url-and-searchbar-color", "toolbar_field_text"],
> +  ["--tabs-border", "toolbar_top_separator"],

As part of this patch, can you please rename `--tabs-border` to `--tabs-border-color`? Right now it is not obvious that this only expects a color value.

	Pulsebot
	Comment 6 • 7 years ago

Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/acede0103156
Add support for toolbar_top/bottom/vertical_separator in the theming API. r=jaws

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 7 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/acede0103156

	Victor Carciu
	Comment 8 • 7 years ago

Is manual testing required on this bug? If yes, please provide some STR and the proper extension(if required) or set the “qe verify-“ flag.

Thanks!

	Will Bamberg [:wbamberg]
	Comment 9 • 7 years ago

I've added these properties and updated the ugly screenshot: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/theme

I wasn't sure how to get "toolbar_vertical_separator" to show up there though, if you can give me a hint I will update it again.