1359062 - Increase contrast of the Downloads Indicator on Windows 8

Status: RESOLVED DUPLICATE of bug 1347543

(Firefox :: Downloads Panel, defect, P1)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Nightly 55.0a1 20170423 on Windows 8.1

At the moment, an active download can be missed e.g.
a) because of the low contrast between filled area (progress) and remaining download: dark blue vs. dark gray.
b) because the download panel might be almost (completely?) empty if only a small fraction has been downloaded so far.

Please make it more obvious if there are active downloads.

Comment 1

[:Paolo Amadini]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #0)
> At the moment, an active download can be missed e.g.
> a) because of the low contrast between filled area (progress) and remaining
> download: dark blue vs. dark gray.

This is something we tried to address in bug 1338984, but there is a limit to how much we can do with the current assets.

Stephen, is there a new design for the Downloads Indicator in the toolbar, given that the Photon front-end redesign will change all the toolbar icons?

Comment 2

[:Paolo Amadini]

It turns out issue (a) is specific to Windows 8, see attachment 8860950 [details].

Clearing the needinfo about the Photon design since we have a short term fix to make here.

Comment 3

[:Paolo Amadini]

(In reply to KM Lee [:rexboy] from bug 1358880 comment #6)
> The contrast does look much lower than I expected. Seems it's because assets
> under Win8 or later uses a more brighter arrow for normal state of the
> download icon. 
> If we're going to fix it without re-design the icon, with the same approach
> as bug 1338984, it will go to a much brighter color:
> http://webaim.org/resources/contrastchecker/?fcolor=797c80&bcolor=c5def8
> 
> I can try on it and make a specified css rule for win8 or above. Maybe
> adjusting the icon so bright may decrease the quality of the icon a bit.

Sounds good to me, let's see what happens.

Comment 4

[:Paolo Amadini]

[Tracking Requested - why for this release]:
UI regression from bug 1338984, the fix will be simple to uplift.

Comment 5

[Gerry Chang [:gchang]]

Track 54+ as UI regression.

Comment 6

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Attachment: brightness 200%

We need at least brighten the icon by 200% to pass the checker.

The color looks somewhat dazzle to me. I'll post a 170% one for reference if following the checker strictly isn't a must.

Comment 7

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Attachment: Brightness 200%

Comment 8

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Attachment: Brightness 170%

Personally I prefer this one but it's not bright enough to pass the checker.

Comment 9

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Paolo do we need to follow the color checker strictly? if yes I guess we have to send the patch with 200% brightness; otherwise the 170% one looks much comfort to me.

Comment 10

[:Paolo Amadini]

I also believe the 170% version is better. The 200% version seems to change in hue as well, and the value is probably too similar to the background anyways. It doesn't pass the checker, but it's the best we can do with the current assets.

I suppose there will be a new Photon version of the indicator at some point, and it might be able to solve these issues.

Comment 11

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Sorry for late reply. I'm in Photon work week and there are things going on. I'll try if I can send the patch by today or it may be postponed to next Tuesday after I go back to my hometown.

Comment 12

[Nathan Froyd [:froydnj]]

Ping for posting the patch from comment 11.

Comment 13

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Attachment: Bug 1359062 - Increase contrast of the Downloads Indicator on Windows 8.

Review commit: https://reviewboard.mozilla.org/r/140600/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/140600/

Comment 14

[KM Lee [:rexboy] (inactivate after Sep. 22)]

I'm sorry for the delay. Just sent the patch.
Paolo may you review it?
The patch is relatively simple, just override the animation rule if it's windows 8.

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 8868940 [details]
Bug 1359062 - Increase contrast of the Downloads Indicator on Windows 8.

https://reviewboard.mozilla.org/r/140600/#review144598

::: browser/themes/windows/downloads/indicator.css:125
(Diff revision 1)
>    background-image: url("chrome://browser/skin/downloads/download-notification-finish.png");
>    animation-name: downloadsIndicatorNotificationFinish;
>    animation-duration: 1s;
>  }
> +
> +@media (-moz-os-version: windows-win8) {

This conflates OS version with the OS theme being used. Does this change make sense for high contrast themes on Win8? If so, do we need it for other versions of Windows too? How about Linux?

Comment 16

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Attachment: Screenshot after bug 1347543 landed

The patch I wrote has tried to workaround the color of win8 only asset, which is /browser/extensions/pocket/skin/windows/Toolbar-win8.png; But yes, I didn't noticed that this asset has already been removed by a set of unified SVG assets across different OSes in bug 1359062.

So yes, since the main cause is from win8 asset, this bug is already resolved by the unification. Just see the screenshot. We can just dup this bug to bug 1359062. But it's better to qa-verify it anyway since bug 1359062 is not for this specified issue.

Comment 17

[KM Lee [:rexboy] (inactivate after Sep. 22)]

It's bug 1347543 rather than 1359062 above. sorry for the typo.

Comment 18

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Set to duplicate with qe-verify?. See my reason in comment 16.

Comment 19

[:Paolo Amadini]

Since bug 1347543 was fixed in 55 and this bug also affects 54, would it make sense to uplift this fix anyways so we don't regress the Windows 8 experience for one release?

Comment 20

[Dão Gottwald [:dao]]

(In reply to :Paolo Amadini from comment #19)
> Since bug 1347543 was fixed in 55 and this bug also affects 54, would it
> make sense to uplift this fix anyways so we don't regress the Windows 8
> experience for one release?

I don't have a strong opinion on this either way.

Comment 21

[KM Lee [:rexboy] (inactivate after Sep. 22)]

I think it would make sense to make it a version 54 only patch. I can proceed to land it if we want.

To Dao:
But I'm not quite sure about the conflates mentioned in comment 15. The patch aims for all occurrences affected by the following rule in /browser/themes/windows/jar.mn:
> % override chrome://browser/skin/Toolbar.png chrome://browser/skin/Toolbar-win8.png os=WINNT osversion=6.3
So the rule in the patch should make sense in my opinion. Did I missed something?

If you're talking about "high contrast white" mode in windows with the white background, We've discussed similar things in bug 1338984 comment 6 and 7 and we think contrast between the states of the icon is more important than background at that time.

Comment 22

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Currently this fix should only be uplifted to beta without landing to Nightly.
Although we tagged it UI-Regression P1, but we’ve figured out that it affects only version 54 on windows 8, and the release date is just in a couple of days. I'm a little bit worry if we have enough time for running both reviewing and QA. Gerry how do you think from Release manager's point of view?

Comment 23

[Dão Gottwald [:dao]]

(In reply to KM Lee [:rexboy] from comment #21)
> I think it would make sense to make it a version 54 only patch. I can
> proceed to land it if we want.
> 
> To Dao:
> But I'm not quite sure about the conflates mentioned in comment 15. The
> patch aims for all occurrences affected by the following rule in
> /browser/themes/windows/jar.mn:
> > % override chrome://browser/skin/Toolbar.png chrome://browser/skin/Toolbar-win8.png os=WINNT osversion=6.3
> So the rule in the patch should make sense in my opinion. Did I missed
> something?
> 
> If you're talking about "high contrast white" mode in windows with the white
> background, We've discussed similar things in bug 1338984 comment 6 and 7
> and we think contrast between the states of the icon is more important than
> background at that time.

I'm not sure I understand what you're saying. Toolbar-win8.png is not used in high contrast mode, but your patch *would* affect high contrast mode on Windows 8. Was this intentional?

Comment 24

[KM Lee [:rexboy] (inactivate after Sep. 22)]

(In reply to Dão Gottwald [::dao] from comment #23)
> 
> I'm not sure I understand what you're saying. Toolbar-win8.png is not used
> in high contrast mode, but your patch *would* affect high contrast mode on
> Windows 8. Was this intentional?

No it wouldn't -- high contrast mode goes to toolbar[brighttext] rule which should override the media query by another animation called indicatorArrowProgressDark{}. I can do a double confirm though.

Comment 25

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Attachment: screenshot for the patch - high contrast mode (regular)

Screenshot of High contrast mode (regular) for the patch. The new rule is not applied on it. If you have it applied then something should be wrong and I'll find out why.

Comment 26

[Dão Gottwald [:dao]]

Have you checked high contrast white?

Comment 27

[Peter Chang[:pchang]]

Rex, are we going to fix this issue in FF 54 before beta shipment?

Comment 28

[Gerry Chang [:gchang]]

(In reply to KM Lee [:rexboy] from comment #22)
> Currently this fix should only be uplifted to beta without landing to
> Nightly.
> Although we tagged it UI-Regression P1, but we’ve figured out that it
> affects only version 54 on windows 8, and the release date is just in a
> couple of days. I'm a little bit worry if we have enough time for running
> both reviewing and QA. Gerry how do you think from Release manager's point
> of view?

Hi Rex, 
I agree with your point. It's very late in Beta54 and we are about to go to RC week. I prefer not to fix this in beta.