Closed Bug 1022550 Opened 5 years ago Closed 3 years ago

Downloads panel's icons look disabled in High Contrast mode

Categories

(Firefox :: Downloads Panel, defect)

All
Windows
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 53
Tracking Status
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- verified
firefox53 --- verified

People

(Reporter: Unfocused, Assigned: selee)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [CHE-MVP][CHE-BUG])

Attachments

(2 files, 1 obsolete file)

Attached image Screenshot (obsolete) —
The downloads panel uses icons (cancel, restart, open folder) that aren't very legible when using High Contrast mode. they should have higher contrast with the background. See attached screenshot.
Flags: firefox-backlog+
Depends on: 1022551
No longer depends on: 1022551
Priority: -- → P3
Will be easier to fix once the icons are converted to SVG.
Depends on: 1289139
Priority: P3 → P4
OS: Windows 8.1 → Windows
Hardware: x86_64 → All
Whiteboard: p=2
Version: 30 Branch → Trunk
So bug 1289139 fixed the contrast, but by using graytext the icons now look disabled. This is more obvious in HC mode because that maps graytext to a very distinctive color (e.g. green whereas normal text would be yellow).

I'm morphing this bug to reflect that new reality.
Blocks: 1289139
No longer depends on: 1289139
Keywords: regression
Priority: P4 → --
Summary: Downloads panel's icons need more contrast when using High Contrast mode → Downloads panel's icons look disabled in High Contrast mode
Attached image screenshot
Attachment #8436793 - Attachment is obsolete: true
Maybe we should restrict graytext to Mac and default Windows themes only, and use the normal field text color for all other themes?

Sean, Carol, can you help evaluate this bug?
Flags: needinfo?(selee)
Flags: needinfo?(chuang)
You could also just use the normal text color across the board and slightly reduce the opacity except for non-default Windows.
Hi Dao,

Could you review the patch attachment 8805059 [details] ?
After discussing the color with Carol, using `color: initial` for action buttons is an acceptable and easy way to satisfy High Contrast mode.
The patch is verified in Windows 10, Ubuntu Linux, and MacOSX.
Flags: needinfo?(selee)
Flags: needinfo?(chuang)
Comment on attachment 8805059 [details]
Bug 1022550 - Fix the color of action buttons in Downloads Panel to satisfy High Contrast mode.;

Did you mean 'inherit'? What color does 'initial' resolve to here?
Sorry, it should be `inherit`, and it refers to the design of Dropmarker[1] in Downloads Panel. The patch has been updated.

[1] http://searchfox.org/mozilla-central/rev/4012e61cc38758ffa1f7ce26039173f67b048853/browser/themes/shared/downloads/downloads.inc.css#60
Comment on attachment 8805059 [details]
Bug 1022550 - Fix the color of action buttons in Downloads Panel to satisfy High Contrast mode.;

You definitely need to look for other rules that can now be removed because of this change. And I guess "inherit" is not the default here, and we need it for overriding platform styling, right?
Comment on attachment 8805059 [details]
Bug 1022550 - Fix the color of action buttons in Downloads Panel to satisfy High Contrast mode.;

Looks like this rule from downloads.inc.css should be redundant now:

+@item@[verdict="Malware"]:hover > .downloadButtonArea > .downloadButton {
+  color: inherit;
+}

Bug 1289139 also used graytext in allDownloadsViewOverlay.inc.css. Should probably fix that too.
Attachment #8805059 - Flags: review?(dao+bmo) → review-
Hi Dao, please review this patch with the changes for downloads.inc.css and allDownloadsViewOverlay.inc.css. Thanks.
Attachment #8805059 - Flags: review?(dao+bmo) → review+
Assignee: nobody → selee
Status: NEW → ASSIGNED
Keywords: checkin-needed
need also review + and finished review in mozreview so we can use autolander to land this
Flags: needinfo?(dao+bmo)
Keywords: checkin-needed
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/11c4b40810da
Fix the color of action buttons in Downloads Panel to satisfy High Contrast mode. r=dao
Flags: needinfo?(dao+bmo)
https://hg.mozilla.org/mozilla-central/rev/11c4b40810da
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Whiteboard: [CHE-MVP][CHE-BUG]
Given that bug 1289139 landed on 52, can we consider uplifting this to Aurora?
Comment on attachment 8805059 [details]
Bug 1022550 - Fix the color of action buttons in Downloads Panel to satisfy High Contrast mode.;

(In reply to Ryan VanderMeulen [:RyanVM] from comment #18)
> Given that bug 1289139 landed on 52, can we consider uplifting this to
> Aurora?

We should!

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1289139
[User impact if declined]: The color of download buttons in high contrast themes would be visible, but would look like a disabled button.
[Is this code covered by automated tests?]: No, we don't test front-end CSS
[Has the fix been verified in Nightly?]: Looks like QE hasn't manually verified the fix yet
[Needs manual test from QE? If yes, steps to reproduce]: In a sentence, run a download in high contrast mode and check the color of the action button.
[List of other uplifts needed for the feature/fix]: None that are specifically related to this bug, we may need more uplifts for the feature later
[Is the change risky?]: Limited risk
[Why is the change risky/not risky?]: The CSS rules touched here are only used by this part of the UI
[String changes made/needed]: None
Attachment #8805059 - Flags: approval-mozilla-aurora?
To clarify, the fix is for download action buttons inside the Downloads Panel.
Comment on attachment 8805059 [details]
Bug 1022550 - Fix the color of action buttons in Downloads Panel to satisfy High Contrast mode.;

fix download panel buttons vs high contrast mode interaction, for aurora52
Attachment #8805059 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I have reproduced this issue using Firefox 32.0a1 (2014.06.09) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Firefox 52.0a2 (2017.01.18) and Firefox 53.0a1 (2017.01.17) on Win 8.1 x64.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.