Closed
Bug 1396062
Opened 7 years ago
Closed 7 years ago
Loading indicator invisible on blue window title
Categories
(Firefox :: Theme, defect, P1)
Firefox
Theme
Tracking
()
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.
Updated•7 years ago
|
Flags: qe-verify?
Priority: -- → P3
Whiteboard: [photon-animation][triage] → [reserve-photon-animation]
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
mozreview-review |
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.
Updated•7 years ago
|
Iteration: --- → 57.3 - Sep 19
Priority: P3 → P1
Comment 3•7 years ago
|
||
mozreview-review |
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)
Comment 4•7 years ago
|
||
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 ?
Comment 5•7 years ago
|
||
(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.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8905539 -
Attachment is obsolete: true
Assignee | ||
Comment 7•7 years ago
|
||
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)
Comment 8•7 years ago
|
||
(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)
Comment 9•7 years ago
|
||
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)
Assignee | ||
Comment 10•7 years ago
|
||
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)
Comment 11•7 years ago
|
||
(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)
Assignee | ||
Comment 12•7 years ago
|
||
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 13•7 years ago
|
||
mozreview-review |
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+
Comment 14•7 years ago
|
||
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)
Comment hidden (mozreview-request) |
Comment 16•7 years ago
|
||
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
Updated•7 years ago
|
Flags: qe-verify? → qe-verify+
QA Contact: stefan.georgiev
Comment 17•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Comment 18•7 years ago
|
||
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
Updated•7 years ago
|
Flags: qe-verify+
Comment 19•7 years ago
|
||
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.
Description
•