Closed Bug 1387582 Opened 7 years ago Closed 7 years ago

Allow modifying the toolbar text color separately from the global text color

Categories

(WebExtensions :: Frontend, defect, P5)

defect

Tracking

(firefox57 verified)

VERIFIED FIXED
mozilla57
Tracking Status
firefox57 --- verified

People

(Reporter: ntim, Assigned: ntim)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: [design-decision-needed][vivaldifox-blockers])

Attachments

(2 files)

You can set the global text color (colors.textcolor) and a toolbar background (colors.toolbar), but you can't set the text color of the toolbars (colors.toolbartext?).
Component: Theme → WebExtensions: Frontend
Product: Firefox → Toolkit
Whiteboard: [design-decision-needed]
Priority: -- → P5
Tbh, it doesn't make sense to reject this proposal since you can already change the toolbar background.

If you have a black toolbar background, you should be able to change the corresponding text color to white.
Blocks: 1386004
Do we do this in a theme that we're shipping with the browser?
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2)
> Do we do this in a theme that we're shipping with the browser?

Yes, the default theme (on macOS at least) has a tabbar with a dark background (and white text), and a navbar with a light background (and black text).

Atm, the theming API doesn't allow implementing the default theme, the closest you can get is:

{
  colors: {
    toolbar: "#f9f9fa",
    textcolor: "#000",
    accentcolor: "#0c0c0d"
  }
}

With that you would get a dark tab bar, but the text in the tab bar would be black.

You could also change textcolor to be white, but then the navbar would have white on light gray text.

Which is why there should be a way of modifying one text color independently from the other.
Attachment #8902658 - Flags: review?(dao+bmo)
Assignee: nobody → ntim.bugs
Whiteboard: [design-decision-needed] → [design-decision-needed][vivaldifox-blockers]
Comment on attachment 8902658 [details]
Bug 1387582 - Add toolbar_text color property to theming API.

https://reviewboard.mozilla.org/r/174330/#review181330

API parts look good to me!

::: browser/themes/linux/browser.css:61
(Diff revision 2)
>  }
>  
>  #navigator-toolbox > toolbar:not(#toolbar-menubar):not(#TabsToolbar) {
>    background-color: var(--toolbar-bgcolor);
>    background-image: var(--toolbar-bgimage);
> +  color: var(--toolbar-color, inherit);

I'm not 100% sure that this does what we want/ need wrt the `inherit` statement (since it rarely has in my experience), so it's good that you flagged Dão to look at this.
Attachment #8902658 - Flags: review?(mdeboer) → review+
Hi Dão, I would like to land this in the 57 cycle as it is a blocker for my extension. Do you think you could review this soon?
Flags: needinfo?(dao+bmo)
Comment on attachment 8902658 [details]
Bug 1387582 - Add toolbar_text color property to theming API.

https://reviewboard.mozilla.org/r/174330/#review185430
Attachment #8902658 - Flags: review?(dao+bmo) → review+
Thank you!
Flags: needinfo?(dao+bmo)
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7bda9cc7b798
Add toolbar_text color property to theming API. r=dao,mikedeboer
https://hg.mozilla.org/mozilla-central/rev/7bda9cc7b798
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
I've updated https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/theme with these new properties, but let me know if we need anything else.
Flags: needinfo?(ntim.bugs)
Looks mostly good to me. Here are a few comments:

- "the background color for the toolbar." The color applies to multiple toolbars (the navigation bar, the bookmarks bar, and the selected tab).

- I think it would be helpful to have a small sketch of where colors apply
Flags: needinfo?(ntim.bugs)
Attached image toolbar color.gif
I can reproduce this issue on Firefox 57.0a1 (20170803134456) under Wind 10 64-bit. 

This issue is verified as fixed on Firefox 57.0b11 (20171023180840) under Wind 10 64-bit and Mac OS X 10.13.

See attached gif.
Status: RESOLVED → VERIFIED
> - I think it would be helpful to have a small sketch of where colors apply

I've made an example with a screenshot. I hope this covers it well enough:

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/theme#example-screenshot
(In reply to Will Bamberg [:wbamberg] from comment #15)
> > - I think it would be helpful to have a small sketch of where colors apply
> 
> I've made an example with a screenshot. I hope this covers it well enough:
> 
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/
> theme#example-screenshot

Very useful, thanks!
Blocks: themingapi-chrome
No longer blocks: themingapi
Blocks: themingapi-more-ui
No longer blocks: themingapi-chrome
Depends on: 1400575
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: