Add internal Google Chrome separator color properties

RESOLVED FIXED in Firefox 58

Status

enhancement
P5
normal
RESOLVED FIXED
2 years ago
11 months ago

People

(Reporter: mikedeboer, Assigned: ntim)

Tracking

(Blocks 1 bug, {dev-doc-complete})

unspecified
mozilla58
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox58 fixed)

Details

(Whiteboard: triaged)

Attachments

(1 attachment)

Reporter

Description

2 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.

Updated

2 years ago
Severity: normal → enhancement
Priority: -- → P5
Assignee

Updated

2 years ago
Assignee: nobody → ntim.bugs
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
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+
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Keywords: dev-doc-needed

Comment 6

2 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
https://hg.mozilla.org/mozilla-central/rev/acede0103156
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58

Comment 8

2 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!
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.
Assignee

Updated

a year ago
Depends on: 1420254
Assignee

Updated

a year ago
Flags: needinfo?(ntim.bugs) → qe-verify-
Assignee

Updated

a year ago
Blocks: themingapi-more-ui
No longer blocks: themingapi-chrome
Summary: Add Google Chrome border/ separator color properties → Add internal Google Chrome separator color properties
Assignee

Updated

a year ago
Depends on: 1423762

Updated

11 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.