Allow setting the tab line color

RESOLVED FIXED in Firefox 60

Status

enhancement
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: mconca, Assigned: ntim)

Tracking

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

59 Branch
mozilla60
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

a year ago
It would be nice to independently style the line at the top of the active tab.

Right now, it appears that the tab line color inherits the color set for accentcolor. Some themes that use background images set both the accentcolor and text color to the same value, so that both the tab line and tab text are visible.

Unfortunately, other add-ons that try to "do the right thing" and read the accent color and text color and use those values end up "invisible" because those properties are set to the same value.

The largest example of this is trying to use Martell's Quantum Lights Theme (https://addons.mozilla.org/en-US/firefox/addon/quantum-lights-dynamic/) with the Tree Style Tabs add-on (https://addons.mozilla.org/en-US/firefox/addon/tree-style-tab/).  Martell uses a background image and sets both the accentcolor and textcolor to white, so both will show up against his background.

Tree Style Tabs tries to use the accentcolor and textcolor of the current theme to render its tabs, but the tabs are unreadable since those values are exactly the same.

Allowing themes to set the tab line color separately permits the line to show up against background images while simultaneously permitting contrasting accent and text colors so that other extensions don't break.
This sounds like a good-next-bug to me, not terribly complicated. :dao and/ or :johannh would be able to provide help, if necessary, and review patches.
Reporter

Comment 2

a year ago
:mconley I'm wondering if we could get this added to the Dark Themes Darkening project for MSU. It seems very similar to a lot of the bugs they are already working on.
Flags: needinfo?(mconley)
I'm helping to mentor the students, but I'd say I'm less able to say with certainty what should and should not be exposed via our Theme API. This sounds reasonable to me, but we might want a third opinion.

dao, what say you? Does this sound like a reasonable thing to expose via the WebExtension API? Is this line at the top of the tab something that could be covered by a pre-existing variable? Do we foresee any future-proofing difficulties if the UI were to change in 5 years time and the line doesn't exist anymore?
Flags: needinfo?(mconley) → needinfo?(dao+bmo)
(In reply to Mike Conley (:mconley) (:⚙️) (Totally backlogged on reviews and needinfos) from comment #3)
> I'm helping to mentor the students, but I'd say I'm less able to say with
> certainty what should and should not be exposed via our Theme API. This
> sounds reasonable to me, but we might want a third opinion.
> 
> dao, what say you? Does this sound like a reasonable thing to expose via the
> WebExtension API?

Yeah, we could.

> Is this line at the top of the tab something that could be
> covered by a pre-existing variable?

Not really. Our options right now are to use the accent color (we do this as of bug 1388138), the text color, the OS-provided highlight color, or a fixed photon color.

> Do we foresee any future-proofing
> difficulties if the UI were to change in 5 years time and the line doesn't
> exist anymore?

That's not a problem. If we decided to remove this UI element at some point, the theme property would just stop doing anything.
Flags: needinfo?(dao+bmo)
Assignee

Updated

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

Updated

a year ago
Summary: Allow setting of the tab line color → Allow setting the tab line color
Comment hidden (mozreview-request)
Assignee

Comment 7

a year ago
mozreview-review
Comment on attachment 8954976 [details]
Bug 1439734 - Allow WebExtension themes to set the tab line color.

https://reviewboard.mozilla.org/r/224162/#review230482

::: toolkit/components/extensions/ext-theme.js:151
(Diff revision 2)
>            this.lwtStyles.icon_color = cssColor;
>            break;
>          case "icons_attention":
>            this.lwtStyles.icon_attention_color = cssColor;
>            break;
> +        case "selected_tab_line":

Note to self: Rename to tab_selected_line or simply tab_line to be consistent with tab_background_text
Comment on attachment 8954976 [details]
Bug 1439734 - Allow WebExtension themes to set the tab line color.

https://reviewboard.mozilla.org/r/224162/#review230800

::: toolkit/components/extensions/test/browser/browser_ext_themes_tab_line.js:12
(Diff revision 2)
> +  const TAB_LINE_COLOR = "#9400ff";
> +  let extension = ExtensionTestUtils.loadExtension({
> +    manifest: {
> +      "theme": {
> +        "images": {
> +          "headerURL": "image1.png",

headerURL is now optional so you can remove it from the test.

::: toolkit/components/extensions/test/browser/browser_ext_themes_tab_line.js:21
(Diff revision 2)
> +    files: {
> +      "image1.png": BACKGROUND,
> +    },

Ditto.
Attachment #8954976 - Flags: review?(jaws) → review+
Comment hidden (mozreview-request)

Comment 10

a year ago
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f7f3f1c04fcd
Allow WebExtension themes to set the tab line color. r=jaws

Comment 11

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f7f3f1c04fcd
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Assignee

Updated

a year ago
Keywords: dev-doc-needed
:ntim, I think you have updated the docs page and data for this, so if you are happy with the screenshots we can mark it dev-doc-complete.
Flags: needinfo?(ntim.bugs)
Assignee

Updated

a year ago
Flags: needinfo?(ntim.bugs)

Updated

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