Closed Bug 1400297 Opened 2 years ago Closed 2 years ago

Too low contrast for netInfoHeadersGroup in JSON Viewer

Categories

(DevTools :: JSON Viewer, defect)

57 Branch
defect
Not set

Tracking

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57 fixed, firefox58 fixed)

RESOLVED FIXED
Firefox 58
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: Oriol, Assigned: Oriol)

References

(Blocks 1 open bug)

Details

(Keywords: access, polish, regression)

Attachments

(1 file)

Load data:application/json,[1,2,3]
Go to Headers tab

Result: "Response Headers" and "Request Headers" have awful contrast.

Currently, for light theme, http://webaim.org/resources/contrastchecker/?fcolor=B1B1B3&bcolor=FFFFFF
 - Foreground: #B1B1B3
 - Background: #FFFFFF
 - Contrast ratio: 2.14:1

Previously, for light theme, http://webaim.org/resources/contrastchecker/?fcolor=585959&bcolor=FFFFFF
 - Foreground: #585959
 - Background: #FFFFFF
 - Contrast ratio: 7.03:1

Currently, for dark theme, http://webaim.org/resources/contrastchecker/?fcolor=737373&bcolor=0C0C0D
 - Foreground: #737373
 - Background: #0C0C0D
 - Contrast ratio: 4.12:1

Previously, for dark theme, http://webaim.org/resources/contrastchecker/?fcolor=B6BABF&bcolor=393F4C
 - Foreground: #B6BABF
 - Background: #393F4C
 - Contrast ratio: 5.41:1
Gabriel, were these new --theme-body-color-alt colors intentional? Should JSON Viewer and Network devtools be using some other color variable instead of --theme-body-color-alt?
Flags: needinfo?(gl)
Forwarding this to Honza to fix. This basically comes down to either using the Accordion component or applying the same styles as the Accordion component.
Flags: needinfo?(gl) → needinfo?(odvarko)
@Oriol: Can you please look at this?

Here is the accordion component styles Gabriel mentioned:
http://searchfox.org/mozilla-central/source/devtools/client/inspector/layout/components/Accordion.css

We should style Net monitor and JSON Viewer the same way.

Honza
Flags: needinfo?(odvarko) → needinfo?(oriol-bugzilla)
But just the color? Also the background? All accordion styles? cursor:pointer and a different color for :hover can be confusing since the JSON headers are not clickable.
Flags: needinfo?(oriol-bugzilla) → needinfo?(odvarko)
Instead of styling it, I think we should just do this right and use the Accordion component.
(In reply to Gabriel [:gl] (ΦωΦ) from comment #5)
> Instead of styling it, I think we should just do this right and use the
> Accordion component.
Gabriel, I agree that it would be better, but this might take time since JSON Viewer is still based on AMD (asynchronous modules). Using the Accordion component would force us to turn that component into AMD.
Would that be ok for you?

I believe that the proper solution would be to use Webpack for JSONViewer. Before that happens we can just fix the colors here for the time being.

Honza
Flags: needinfo?(odvarko) → needinfo?(gl)
(In reply to Jan Honza Odvarko [:Honza] from comment #6)
> (In reply to Gabriel [:gl] (ΦωΦ) from comment #5)
> > Instead of styling it, I think we should just do this right and use the
> > Accordion component.
> Gabriel, I agree that it would be better, but this might take time since
> JSON Viewer is still based on AMD (asynchronous modules). Using the
> Accordion component would force us to turn that component into AMD.
> Would that be ok for you?
> 
> I believe that the proper solution would be to use Webpack for JSONViewer.
> Before that happens we can just fix the colors here for the time being.
> 
> Honza

Ah, I made a huge mistake of not fully reading this bug. For the longest time, I thought this was for the network monitor instead of the JSON viewer. 

If this is the case, we should just fix the color css for now.
Flags: needinfo?(gl)
Contrast can be an a11y issue so I'm hoping we can keep it on the radar as a regression (maybe to fix for 58)
Keywords: access
Just a small color fix.
Assignee: nobody → oriol-bugzilla
Status: NEW → ASSIGNED
Comment on attachment 8914370 [details]
Bug 1400297 - Increase color contrast for JSON Viewer and Netmonitor headers.

https://reviewboard.mozilla.org/r/185644/#review190936

Yes, this is nice improvement, thanks!

R+

Honza
Attachment #8914370 - Flags: review?(odvarko) → review+
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/08b8284f7cb7
Increase color contrast for JSON Viewer and Netmonitor headers. r=Honza
https://hg.mozilla.org/mozilla-central/rev/08b8284f7cb7
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Should we consider uplifting this to Beta for the sake of our DevEdition users?
Flags: needinfo?(oriol-bugzilla)
Comment on attachment 8914370 [details]
Bug 1400297 - Increase color contrast for JSON Viewer and Netmonitor headers.

Approval Request Comment
[Feature/Bug causing the regression]: Regression from bug 1398880
[User impact if declined]: Accessibility problems (WCAG-AA failures). People with color vision deficiency may not be able to read the affected text.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: I have manually verified this in Nightly.
[Needs manual test from QE? If yes, steps to reproduce]: See comment 0.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Very low risk.
[Why is the change risky/not risky?]: Just a small CSS color change.
[String changes made/needed]: None
Flags: needinfo?(oriol-bugzilla)
Attachment #8914370 - Flags: approval-mozilla-beta?
Comment on attachment 8914370 [details]
Bug 1400297 - Increase color contrast for JSON Viewer and Netmonitor headers.

Extremely low risk polish fix, Beta57+
Attachment #8914370 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I have reproduced this bug with Nightly 57.0a1 (2017-09-15) on Windows 7, 64 Bit!

This bug's fix is Verified with latest Beta & latest Nightly!

Build   ID    20171102181127
User Agent    Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0

Build   ID    20171106100122
User Agent    Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
QA Whiteboard: [bugday-20171101]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.