1207565 - [Ubuntu] Make tab sound visual indicator more visible

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Ada [:adalucinet]]

Affected platform: Ubuntu.

Preconditions: Make sure you have opened several tabs (~16 tabs) so the "List all tabs" button is displayed.
1. Launch Firefox.
2. Open several websites with audio/video content.
3. Click on the "List all tabs" button. 

Expected result: The tab sound icon is clearly visible in the tabs list drop-down, for the tabs playing audio.

Actual result: The tab sound icon appears in the tabs list drop-down, but is barely visible with Ubuntu's default theme. Same issue when a tab with audio/video content is in focus. Please see the following screen recording → https://goo.gl/I5ccWW

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Ehsan, could you have a look to this? Thanks

Comment 2

[(no longer active)]

So I can't just use #TabsToolbar[brighttext] in my selector because the brighttext attribute is set based on the color of the toolbar, not that of the menu item.  So while using #TabsToolbar[brighttext] would fix this on Linux where the two colors are the same, it would regress it on OS X dev edition which has a dark toolbar and a bright menu item.

Jared, what's a better selector to use here?

Comment 3

[:Gijs (he/him)]

I don't think there's a selector available that detects the menu text that's in use. You'd need to write some JS that does essentially what this code:

https://dxr.mozilla.org/mozilla-central/rev/f1dffc8682fbba463cb4bb305f293ddcccbc20b4/browser/base/content/browser.js#7920-7952

is doing, and then use that attribute, I think.

Comment 4

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #3)
> I don't think there's a selector available that detects the menu text that's
> in use. You'd need to write some JS that does essentially what this code:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> f1dffc8682fbba463cb4bb305f293ddcccbc20b4/browser/base/content/browser.
> js#7920-7952
> 
> is doing, and then use that attribute, I think.

Please don't. The SVG itself can just use the MenuText color.

Comment 5

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #4)
> (In reply to :Gijs Kruitbosch from comment #3)
> > I don't think there's a selector available that detects the menu text that's
> > in use. You'd need to write some JS that does essentially what this code:
> > 
> > https://dxr.mozilla.org/mozilla-central/rev/
> > f1dffc8682fbba463cb4bb305f293ddcccbc20b4/browser/base/content/browser.
> > js#7920-7952
> > 
> > is doing, and then use that attribute, I think.
> 
> Please don't. The SVG itself can just use the MenuText color.

Can it? It looks like it has hardcoded colors, and has hover/active states which differ only by color...

Comment 6

[Dão Gottwald [:dao]]

It does have hardcoded color codes, but we have full control over that and can use platform colors as needed.

The icon in the tabs menu doesn't have a hover state. For the icon in the tabs we also change the opacity on hover.

Comment 7

[(no longer active)]

Can I just use MenuText without the second part of comment 6 in this bug?

Comment 8

[Dão Gottwald [:dao]]

Sure, but file a followup on improving the icon in tabs too?

Comment 9

[(no longer active)]

Of course.

Comment 10

[(no longer active)]

Attachment: Use the MenuText color for the audio indicator in the all tabs menu

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 8666377 [details] [diff] [review]
Use the MenuText color for the audio indicator in the all tabs menu

Might also make sense to change the icon color when the menu item is active. Unfortunately the text color used there isn't consistent across OSes. -moz-menuhovertext on Windows and Linux, -moz-mac-menutextselect on OS X.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/40a416eadddf

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/40a416eadddf