Closed Bug 1396062 Opened 7 years ago Closed 7 years ago

Loading indicator invisible on blue window title

Categories

(Firefox :: Theme, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 57
Iteration:
57.3 - Sep 19
Tracking Status
firefox57 --- verified

People

(Reporter: jaws, Assigned: jaws)

References

Details

(Whiteboard: [reserve-photon-animation])

Attachments

(2 files, 1 obsolete file)

Spun out from bug 1378188,

See https://bugzilla.mozilla.org/attachment.cgi?id=8902776

(In reply to Peter L Jones from bug 1378188 comment #6)
> Created attachment 8902776 [details]
> Example of blue and grey tabs in default theme
> 
> Theme is Default, as it was in the original report.
> Firefox is 57.0a1 (2017-08-30) (64-bit).
> 
> The blue dot on a pale tab cycling back and forth is visible OK.  However,
> on a blue tab, it's only the tiny highlight is almost unnoticeable.

(In reply to Peter L Jones from bug 1378188 comment #7)
> If the active tab (grey) uses the inactive tab colour (blue) for the
> spinner, the inactive tab (blue) should use the active tab colour (grey) for
> the spinner.  Of course, the highlight would need taking care of.
Flags: qe-verify?
Priority: -- → P3
Whiteboard: [photon-animation][triage] → [reserve-photon-animation]
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Comment on attachment 8905539 [details]
Bug 1396062 - If the contrast between the Windows accent color and the loading throbber isn't high enough, use the accentcolortext as the loading throbber fill color.

https://reviewboard.mozilla.org/r/177338/#review182350

::: browser/themes/shared/tabs.inc.css:28
(Diff revision 1)
>  
>  :root:-moz-lwtheme {
>    --tab-line-color: var(--lwt-accent-color);
>  }
>  
> +:root[highcontrast-winaccentcolor] .tabbrowser-tab:not([visuallyselected=true]) {

I couldn't think of a better name than "highcontrast-winaccentcolor", but I'm also not fond of it. Other suggestions welcome.
Iteration: --- → 57.3 - Sep 19
Priority: P3 → P1
Comment on attachment 8905539 [details]
Bug 1396062 - If the contrast between the Windows accent color and the loading throbber isn't high enough, use the accentcolortext as the loading throbber fill color.

https://reviewboard.mozilla.org/r/177338/#review182356

::: browser/themes/shared/tabs.inc.css:28
(Diff revision 1)
>  
>  :root:-moz-lwtheme {
>    --tab-line-color: var(--lwt-accent-color);
>  }
>  
> +:root[highcontrast-winaccentcolor] .tabbrowser-tab:not([visuallyselected=true]) {

I think this also needs to check tabsintitlebar, and probably also whether we're using a lightweight theme or something else that overrides the accent colour in terms of the background of the background tabs?
Attachment #8905539 - Flags: review?(gijskruitbosch+bugs)
Would it be possible to have a more general fix that works for dark lightweight themes too ?


#TabsToolbar[brighttext] .tabbrowser-tab:not([visuallyselected=true]) {
  fill: /* some lighter blue color */;
}

Possible candidates would be Teal 50/60 or Blue 40.

It would also fix the issue in this bug as well: if someone selects teal as accent color, there's no [brighttext] attribute, and Firefox will use blue. If someone selects blue as accent color, Firefox will use teal. This guarantees the throbber to always be visible ?
(In reply to Tim Nguyen :ntim from comment #4)
> Would it be possible to have a more general fix that works for dark
> lightweight themes too ?
> 
> 
> #TabsToolbar[brighttext] .tabbrowser-tab:not([visuallyselected=true]) {
>   fill: /* some lighter blue color */;
> }

This assumes that the blue background gets `[brighttext]` and I assume that that isn't necessarily the case, depending on the shade of blue.
Attachment #8905539 - Attachment is obsolete: true
Amy said she was OK with using white as the fill color for brighttext, but looking at http://design.firefox.com/photon/visual/color.html I don't think white will be the right color.

Amy, what do you think about using Teal 50 instead of white?
Flags: needinfo?(amlee)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7)
> Amy said she was OK with using white as the fill color for brighttext, but
> looking at http://design.firefox.com/photon/visual/color.html I don't think
> white will be the right color.
> 
> Amy, what do you think about using Teal 50 instead of white?

NI Eric for feedback on this
Flags: needinfo?(amlee) → needinfo?(epang)
So does this actually address the issue for lighter shades of blue that don't get [brighttext] (cf. comment #5), or do no such shades of blue exist?
Flags: needinfo?(jaws)
I tested with various shades of blue. Teal 50 isn't perfect, but it does provide AAA contrast with black. The alternative is using white as the color.

rgb(0,183,195) doesn't get [brighttext]
rgb(0,120,215) get's [brighttext]
rgb(1,177,215) seems to be the point where we remove [brighttext]

The contrast between #0A84FF and #00feff isn't that great. We may be forced to use #fff, but as #fff is not in the color palette I wanted to double-check with UX about using white here.
Flags: needinfo?(jaws)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #10)
> I tested with various shades of blue. Teal 50 isn't perfect, but it does
> provide AAA contrast with black. The alternative is using white as the color.
> 
> rgb(0,183,195) doesn't get [brighttext]
> rgb(0,120,215) get's [brighttext]
> rgb(1,177,215) seems to be the point where we remove [brighttext]
> 
> The contrast between #0A84FF and #00feff isn't that great. We may be forced
> to use #fff, but as #fff is not in the color palette I wanted to
> double-check with UX about using white here.

Are you able to attach a screencap of the options?
Flags: needinfo?(jaws)
This shows the default color choices that Windows 10 offers. But also there is a checkbox to determine the color from the desktop background and another button to use a custom color.

To determine if white text ("brighttext") would be used on the toolbar, you would need to get the RGB of the color, then do the following two steps:

1. Calculate the luminance. It is defined by: 0.2125 * r + 0.7154 * g + 0.0721 * b;
2. If the luminance is less than or equal to 110, then we will use dark text (black). Otherwise we will use bright text (white).
Flags: needinfo?(jaws)
Comment on attachment 8907823 [details]
Bug 1396062 - Use White as the fill color for the tab loading indicator when the tabs toolbar has brighttext.

https://reviewboard.mozilla.org/r/179522/#review184978

OK, tested a bit, and I think this makes sense, even if white would probably provide more contrast. I'll leave the final decision up to you + UX (you can carry over r=me if you swap colours, no need to ask for re-review).
Attachment #8907823 - Flags: review?(gijskruitbosch+bugs) → review+
Teal 50 is used more for attention, alerts and messaging.  Because of this reason and visual esthetics  I recommend we use white as the colour.
Flags: needinfo?(epang)
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/065ed2f128fe
Use White as the fill color for the tab loading indicator when the tabs toolbar has brighttext. r=Gijs
Flags: qe-verify? → qe-verify+
QA Contact: stefan.georgiev
https://hg.mozilla.org/mozilla-central/rev/065ed2f128fe
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I was able to reproduce this issue with the following Nightly build (20170901150353).

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 (20170915100121)

This issue is verified as fixed with the latest Nightly build on 9/15/2017.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Thanks, this does look better.

(Off topic, the tab icon could maybe do with a similar soft border on inactive tabs -  site designers seem to assume a pale background.)
You need to log in before you can comment on or make changes to this bug.