Closed Bug 1412595 Opened 7 years ago Closed 7 years ago

Implement colors.bookmark_text for themes

Categories

(WebExtensions :: Frontend, enhancement, P5)

enhancement

Tracking

(firefox57 wontfix, firefox58 verified, firefox59 verified)

VERIFIED FIXED
mozilla58
Tracking Status
firefox57 --- wontfix
firefox58 --- verified
firefox59 --- verified

People

(Reporter: ntim, Assigned: ntim)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

colors.bookmark_text is supported in chrome.

I think it makes sense to support bookmark_text as an alias for toolbar_text as it work better for compatibility with chrome.
Assignee: nobody → ntim.bugs
Severity: normal → enhancement
Priority: -- → P5
Comment on attachment 8923765 [details]
Bug 1412595 - Implement colors.bookmark_text as alias for colors.toolbar_text.

https://reviewboard.mozilla.org/r/194912/#review201540

::: toolkit/components/extensions/ext-theme.js:137
(Diff revision 2)
>          case "toolbar_text":
> +        case "bookmark_text":
> +          this.lwtStyles.toolbar_text = cssColor;

But which color should win if both are specified? Right now the code will treat the *last* specified as the used color, this may be confusing to people who port over their theme from Chrome.
Attachment #8923765 - Flags: review?(jaws) → review-
Attachment #8923765 - Flags: review- → review?(jaws)
Comment on attachment 8923765 [details]
Bug 1412595 - Implement colors.bookmark_text as alias for colors.toolbar_text.

https://reviewboard.mozilla.org/r/194912/#review201952
Attachment #8923765 - Flags: review?(jaws) → review+
Keywords: dev-doc-needed
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b4a8921258e0
Implement colors.bookmark_text as alias for colors.toolbar_text. r=jaws
https://hg.mozilla.org/mozilla-central/rev/b4a8921258e0
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Attached image Bug1412595.gif
I can reproduce this issue on Firefox 57.0 (20171112125346) under Wind 7 64-bit.

This issue is verified as fixed on Firefox 59.0a1 (20171120100042) and Firefox 58.0b4 (20171115114231) under Wind 7 64-bit and Mac OS X 10.13. 

Using "bookmark_text": "#7206F5" will color the text of the tab in focus and the toolbar icons.

Please see the attached video.
Status: RESOLVED → VERIFIED
I noticed that if you use bookmark_text and toolbar_text together, the color from toolbar_text will be always picked, no matter in what order they are used.

Is this expected? Or I should file another bug?
Flags: needinfo?(ntim.bugs)
Seems like a reasonable behaviour for now. Bug 1415209 will polish this.
Flags: needinfo?(ntim.bugs)
I've updated https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/theme for this, please let me know if this covers it.
Flags: needinfo?(ntim.bugs)
Works for me, thanks!
Flags: needinfo?(ntim.bugs)
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: