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)
Attached image Default colors from Win10 Settings app (deleted) —
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
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.

Attachment

General

Created:
Updated:
Size: