Closed Bug 1347190 Opened 4 years ago Closed 3 years ago

Add internal Google Chrome separator color properties

Categories

(WebExtensions :: Frontend, enhancement, P5)

enhancement

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: mikedeboer, Assigned: ntim)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: triaged)

Attachments

(1 file)

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.
Severity: normal → enhancement
Priority: -- → P5
Assignee: nobody → ntim.bugs
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.
Attachment #8923638 - Flags: review?(jaws) → review+
Keywords: dev-doc-needed
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
https://hg.mozilla.org/mozilla-central/rev/acede0103156
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
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!
Flags: needinfo?(ntim.bugs)
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.
Depends on: 1420254
Flags: needinfo?(ntim.bugs) → qe-verify-
Blocks: themingapi-more-ui
No longer blocks: themingapi-chrome
Summary: Add Google Chrome border/ separator color properties → Add internal Google Chrome separator color properties
Depends on: 1423762
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.