Closed Bug 1396062 Opened 3 years ago Closed 3 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.
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: 3 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.