Implement colors.bookmark_text for themes

VERIFIED FIXED in Firefox 58

Status

P5
enhancement
VERIFIED FIXED
a year ago
9 months ago

People

(Reporter: ntim, Assigned: ntim)

Tracking

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

unspecified
mozilla58
dev-doc-complete

Firefox Tracking Flags

(firefox57 wontfix, firefox58 verified, firefox59 verified)

Details

Attachments

(2 attachments)

(Assignee)

Description

a year ago
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)

Updated

a year ago
Assignee: nobody → ntim.bugs
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Updated

a year ago
Severity: normal → enhancement
status-firefox57: --- → wontfix
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-
(Assignee)

Updated

a year ago
Attachment #8923765 - Flags: review- → review?(jaws)
(Assignee)

Updated

a year ago
Keywords: dev-doc-needed

Comment 5

a year ago
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

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b4a8921258e0
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58

Comment 7

a year ago
Posted 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.

Updated

a year ago
Status: RESOLVED → VERIFIED
status-firefox58: fixed → verified
status-firefox59: --- → verified

Comment 8

a year ago
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)
(Assignee)

Comment 9

a year ago
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)
(Assignee)

Comment 11

a year ago
Works for me, thanks!
Flags: needinfo?(ntim.bugs)
Keywords: dev-doc-needed → dev-doc-complete

Updated

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