Enable the accessibility audit for chrome://net-internals

Enable the accessibility audit for chrome://net-internals

Fixes some tiny bugs - just unlabeled controls and colors that aren't
sufficiently high-contrast.

If any of these fixes are undesirable we can just suppress those errors
instead.

BUG=265583
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=217775

[dmazzoni, 2013-08-02 19:32:36]

Alice, want to make sure this looks sane before I
loop in net-internals owners?

[mmenke, 2013-08-02 19:34:46]

On 2013/08/02 19:32:36, Dominic Mazzoni wrote:
> Alice, want to make sure this looks sane before I
> loop in net-internals owners?

Just thought I'd point out that net-internals is an internal developer page, not
intended public consumption.  Do we care about accessibility auditing for it?

[mmenke, 2013-08-02 19:35:31]

On 2013/08/02 19:34:46, mmenke wrote:
> On 2013/08/02 19:32:36, Dominic Mazzoni wrote:
> > Alice, want to make sure this looks sane before I
> > loop in net-internals owners?
> 
> Just thought I'd point out that net-internals is an internal developer page,
not
> intended public consumption.  Do we care about accessibility auditing for it?

That should be "internal Chrome developer page".  It's also not
internationalized, since it refers to network objects by class name.

[dmazzoni, 2013-08-02 19:51:08]

On 2013/08/02 19:35:31, mmenke wrote:
> On 2013/08/02 19:34:46, mmenke wrote:
> > On 2013/08/02 19:32:36, Dominic Mazzoni wrote:
> > > Alice, want to make sure this looks sane before I
> > > loop in net-internals owners?
> > 
> > Just thought I'd point out that net-internals is an internal developer page,
> not
> > intended public consumption.  Do we care about accessibility auditing for
it?
> 
> That should be "internal Chrome developer page".  It's also not
> internationalized, since it refers to network objects by class name.

Understood.

Alice and I are trying to enable the accessibility audit for all of WebUI. There
are many WebUI pages where this is quite useful. We're going through all of the
errors found and either fixing them in the code, or fixing the audit tool.

Going through net-internals caught one bug in the audit tool which we just
fixed. So it has been useful for us.

[aboxhall1, 2013-08-02 19:57:01]

With regard to accessibility going forward: I don't think there is any area
of UI that is exempt from accessibility considerations. While we might
reasonably assume that all chromium developers will speak enough English to
engage with the codebase, we definitely don't want to exclude developers
who have vision or any other impairments (and indeed we have several such
developers working on the project) if we can avoid it without too much
effort.


On Fri, Aug 2, 2013 at 12:51 PM, <dmazzoni@chromium.org> wrote:

> On 2013/08/02 19:35:31, mmenke wrote:
>
>> On 2013/08/02 19:34:46, mmenke wrote:
>> > On 2013/08/02 19:32:36, Dominic Mazzoni wrote:
>> > > Alice, want to make sure this looks sane before I
>> > > loop in net-internals owners?
>> >
>> > Just thought I'd point out that net-internals is an internal developer
>> page,
>> not
>> > intended public consumption.  Do we care about accessibility auditing
>> for
>>
> it?
>
>  That should be "internal Chrome developer page".  It's also not
>> internationalized, since it refers to network objects by class name.
>>
>
> Understood.
>
> Alice and I are trying to enable the accessibility audit for all of WebUI.
> There
> are many WebUI pages where this is quite useful. We're going through all
> of the
> errors found and either fixing them in the code, or fixing the audit tool.
>
> Going through net-internals caught one bug in the audit tool which we just
> fixed. So it has been useful for us.
>
>
>
https://codereview.chromium.**org/21944004/<https://codereview.chromium.org/2...
>

[aboxhall, 2013-08-03 00:46:07]

lgtm

Nice one!

[dmazzoni, 2013-08-05 16:00:15]

@mmenke: OK, please take a look when you have a chance.
As I said above, if you'd prefer not to make any change
or class of change, we can just suppress the error instead.

[mmenke, 2013-08-05 16:22:39]

On 2013/08/02 19:57:01, aboxhall_google.com wrote:
> With regard to accessibility going forward: I don't think there is any area
> of UI that is exempt from accessibility considerations. While we might
> reasonably assume that all chromium developers will speak enough English to
> engage with the codebase, we definitely don't want to exclude developers
> who have vision or any other impairments (and indeed we have several such
> developers working on the project) if we can avoid it without too much
> effort.

I don't mean to come off as someone opposed to improving accessibility, and I'm
completely fine with these changes, I just question the utility of non-UI
developers spending time dealing with automated accessibility audits
(accessibilityAuditConfig?) on code that's not part of the public Chrome UI.

[mmenke, 2013-08-05 16:23:13]

https://codereview.chromium.org/21944004/diff/1/chrome/browser/resources/net_...
File chrome/browser/resources/net_internals/events_view.css (right):

https://codereview.chromium.org/21944004/diff/1/chrome/browser/resources/net_...
chrome/browser/resources/net_internals/events_view.css:106: color: rgb(235, 0,
0);
Any reason we're using rgb(235,0,0) here and #e00 and rgb(238, 0, 0) elsewhere?

[dmazzoni, 2013-08-05 16:25:33]

On 2013/08/05 16:23:13, mmenke wrote:
>
https://codereview.chromium.org/21944004/diff/1/chrome/browser/resources/net_...
> File chrome/browser/resources/net_internals/events_view.css (right):
> 
>
https://codereview.chromium.org/21944004/diff/1/chrome/browser/resources/net_...
> chrome/browser/resources/net_internals/events_view.css:106: color: rgb(235, 0,
> 0);
> Any reason we're using rgb(235,0,0) here and #e00 and rgb(238, 0, 0)
elsewhere?

The lint checker required it here.

[mmenke, 2013-08-05 16:29:05]

On 2013/08/05 16:25:33, Dominic Mazzoni wrote:
> On 2013/08/05 16:23:13, mmenke wrote:
> >
>
https://codereview.chromium.org/21944004/diff/1/chrome/browser/resources/net_...
> > File chrome/browser/resources/net_internals/events_view.css (right):
> > 
> >
>
https://codereview.chromium.org/21944004/diff/1/chrome/browser/resources/net_...
> > chrome/browser/resources/net_internals/events_view.css:106: color: rgb(235,
0,
> > 0);
> > Any reason we're using rgb(235,0,0) here and #e00 and rgb(238, 0, 0)
> elsewhere?
> 
> The lint checker required it here.

Why?  Not a strong believer in doing things because an automated system said so,
without at least some understanding of why it said so.

[mmenke, 2013-08-05 16:32:37]

On 2013/08/05 16:29:05, mmenke wrote:
> On 2013/08/05 16:25:33, Dominic Mazzoni wrote:
> > On 2013/08/05 16:23:13, mmenke wrote:
> > >
> >
>
https://codereview.chromium.org/21944004/diff/1/chrome/browser/resources/net_...
> > > File chrome/browser/resources/net_internals/events_view.css (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/21944004/diff/1/chrome/browser/resources/net_...
> > > chrome/browser/resources/net_internals/events_view.css:106: color:
rgb(235,
> 0,
> > > 0);
> > > Any reason we're using rgb(235,0,0) here and #e00 and rgb(238, 0, 0)
> > elsewhere?
> > 
> > The lint checker required it here.
> 
> Why?  Not a strong believer in doing things because an automated system said
so,
> without at least some understanding of why it said so.

Not trying to be difficult here, just feel I should understand the reason for
that kind of thing before signing off on it.

[dmazzoni, 2013-08-05 16:36:33]

On Mon, Aug 5, 2013 at 9:29 AM, <mmenke@chromium.org> wrote:

> > Any reason we're using rgb(235,0,0) here and #e00 and rgb(238, 0, 0)
>> elsewhere?
>>
>
>  The lint checker required it here.
>>
>
> Why?  Not a strong believer in doing things because an automated system
> said so,
> without at least some understanding of why it said so.
>
>
https://codereview.chromium.**org/21944004/<https://codereview.chromium.org/2...
>

Here's the warning.

** Presubmit Messages **
Was the CSS checker useful? Send feedback or hate mail to dbeam@chromium.org
.
...
chrome/browser/resources/net_internals/events_view.css:
- Use rgb() over #hex when not a shade of gray (like #333).
    color: #e00; (replace with rgb(238, 0, 0))

[dmazzoni, 2013-08-05 16:39:24]

On 2013/08/05 16:29:05, mmenke wrote:
> > > Any reason we're using rgb(235,0,0) here and #e00 and rgb(238, 0, 0)
> > elsewhere?

Sorry, I might have misunderstood.

The lint checker suggested I use rgb(235, 0, 0) instead of #e00 (syntax).

The choice of rgb(235, 0, 0) was suggested by the accessibility audit as
an alternative to "red" that has sufficiently high contrast according to
the WCAG's formula.

[mmenke1, 2013-08-05 16:42:59]

On 2013/08/05 16:39:24, Dominic Mazzoni wrote:
> On 2013/08/05 16:29:05, mmenke wrote:
> > > > Any reason we're using rgb(235,0,0) here and #e00 and rgb(238, 0, 0)
> > > elsewhere?
> 
> Sorry, I might have misunderstood.
> 
> The lint checker suggested I use rgb(235, 0, 0) instead of #e00 (syntax).
> 
> The choice of rgb(235, 0, 0) was suggested by the accessibility audit as
> an alternative to "red" that has sufficiently high contrast according to
> the WCAG's formula.

But they're different colors:  #E00 is rgb(238, 0, 0), I believe.  There's also
one place where you actually used rgb(238, 0, 0), but in most places you went
with 235 instead.  Not a big deal, but I was just wondering if there's a reason
for the slightly different shades of red.

[mmenke, 2013-08-05 16:45:22]

On 2013/08/05 16:42:59, mmenke (incorrect) wrote:
> On 2013/08/05 16:39:24, Dominic Mazzoni wrote:
> > On 2013/08/05 16:29:05, mmenke wrote:
> > > > > Any reason we're using rgb(235,0,0) here and #e00 and rgb(238, 0, 0)
> > > > elsewhere?
> > 
> > Sorry, I might have misunderstood.
> > 
> > The lint checker suggested I use rgb(235, 0, 0) instead of #e00 (syntax).
> > 
> > The choice of rgb(235, 0, 0) was suggested by the accessibility audit as
> > an alternative to "red" that has sufficiently high contrast according to
> > the WCAG's formula.
> 
> But they're different colors:  #E00 is rgb(238, 0, 0), I believe.  There's
also
> one place where you actually used rgb(238, 0, 0), but in most places you went
> with 235 instead.  Not a big deal, but I was just wondering if there's a
reason
> for the slightly different shades of red.

Anyways, LGTM, regardless of the reason.

[aboxhall1, 2013-08-05 16:45:51]

That's a reasonable concern. I would answer that:
- It really shouldn't take up too much of your time: we started with this
page because fixing it is fairly straightforward, because it's largely a
presentation of text data which is inherently accessible. It makes a good
example case for other, user-facing WebUI pages because it's so simple. If
the maintenance starts to be a problem, we can re-assess.
- As mentioned previously, just because this is not a user-facing page
doesn't mean accessibility concerns don't apply. For example, the recent
thread on the new chromestatus.com included a response from a Chrome
developer who found it inaccessible.

I can definitely understand your reluctance to commit to what looks like a
pointless bureaucratic hoop to jump through. However, in practice it
shouldn't actually add too much work, and if it is, we can easily silence
the audit on this page with a single line of code.

On Mon, Aug 5, 2013 at 9:22 AM, <mmenke@chromium.org> wrote:

> On 2013/08/02 19:57:01, aboxhall_google.com wrote:
>
>> With regard to accessibility going forward: I don't think there is any
>> area
>> of UI that is exempt from accessibility considerations. While we might
>> reasonably assume that all chromium developers will speak enough English
>> to
>> engage with the codebase, we definitely don't want to exclude developers
>> who have vision or any other impairments (and indeed we have several such
>> developers working on the project) if we can avoid it without too much
>> effort.
>>
>
> I don't mean to come off as someone opposed to improving accessibility,
> and I'm
> completely fine with these changes, I just question the utility of non-UI
> developers spending time dealing with automated accessibility audits
> (accessibilityAuditConfig?) on code that's not part of the public Chrome
> UI.
>
>
https://codereview.chromium.**org/21944004/<https://codereview.chromium.org/2...
>

[dmazzoni, 2013-08-05 16:47:07]

On 2013/08/05 16:42:59, mmenke (incorrect) wrote:
> But they're different colors:  #E00 is rgb(238, 0, 0), I believe.  There's
also
> one place where you actually used rgb(238, 0, 0), but in most places you went
> with 235 instead.  Not a big deal, but I was just wondering if there's a
reason
> for the slightly different shades of red.

You're right. The 235 looks like my mistake. It looks like #e00 / rgb(238, 0, 0)
should be used everywhere. I'll fix it.

[mmenke, 2013-08-05 16:51:29]

On 2013/08/05 16:45:51, aboxhall_google.com wrote:
> That's a reasonable concern. I would answer that:
> - It really shouldn't take up too much of your time: we started with this
> page because fixing it is fairly straightforward, because it's largely a
> presentation of text data which is inherently accessible. It makes a good
> example case for other, user-facing WebUI pages because it's so simple. If
> the maintenance starts to be a problem, we can re-assess.
> - As mentioned previously, just because this is not a user-facing page
> doesn't mean accessibility concerns don't apply. For example, the recent
> thread on the new http://chromestatus.com included a response from a Chrome
> developer who found it inaccessible.
> 
> I can definitely understand your reluctance to commit to what looks like a
> pointless bureaucratic hoop to jump through. However, in practice it
> shouldn't actually add too much work, and if it is, we can easily silence
> the audit on this page with a single line of code.

The label/title/div->button changes all seem reasonable to me.  These I
understand the reason for, and seem pretty reasonable and easy to deal with in a
deterministic manner.

It's more the color changes and ignoreSelectors thing that make me a bit
concerned about future maintenance overhead.

> 
> On Mon, Aug 5, 2013 at 9:22 AM, <mailto:mmenke@chromium.org> wrote:
> 
> > On 2013/08/02 19:57:01, http://aboxhall_google.com wrote:
> >
> >> With regard to accessibility going forward: I don't think there is any
> >> area
> >> of UI that is exempt from accessibility considerations. While we might
> >> reasonably assume that all chromium developers will speak enough English
> >> to
> >> engage with the codebase, we definitely don't want to exclude developers
> >> who have vision or any other impairments (and indeed we have several such
> >> developers working on the project) if we can avoid it without too much
> >> effort.
> >>
> >
> > I don't mean to come off as someone opposed to improving accessibility,
> > and I'm
> > completely fine with these changes, I just question the utility of non-UI
> > developers spending time dealing with automated accessibility audits
> > (accessibilityAuditConfig?) on code that's not part of the public Chrome
> > UI.
> >
> >
>
https://codereview.chromium.**org/21944004/%3Chttps://codereview.chromium.org...>
> >

[dmazzoni, 2013-08-05 17:21:29]

On Mon, Aug 5, 2013 at 9:51 AM, <mmenke@chromium.org> wrote:

> It's more the color changes and
>

It seems reasonable to relax the requirements a bit for internal pages, and
only
warn for text that would be extremely hard to read.


> ignoreSelectors thing that make me a bit
> concerned about future maintenance overhead.
>

This seems solvable. The audit could actually print out the exact code to
use, something
like this:

# If you want to suppress this error, add this line of code to your setUp
method:
this.accessibilityAuditConfig().ignoreSelectors(...
# If you want to suppress all errors of this type...
this.accessibilityAuditConfig().ignoreSelectors(...
# If you want to ignore all accessibility errors for this test:
this.accessibilityIssuesAreErrors = false;

- Dominic

[dmazzoni, 2013-08-05 17:24:02]

On Mon, Aug 5, 2013 at 9:47 AM, <dmazzoni@chromium.org> wrote:

> On 2013/08/05 16:42:59, mmenke (incorrect) wrote:
>
>> But they're different colors:  #E00 is rgb(238, 0, 0), I believe.  There's
>>
> also
>
>> one place where you actually used rgb(238, 0, 0), but in most places you
>> went
>> with 235 instead.  Not a big deal, but I was just wondering if there's a
>>
> reason
>
>> for the slightly different shades of red.
>>
>
> You're right. The 235 looks like my mistake. It looks like #e00 / rgb(238,
> 0, 0)
> should be used everywhere. I'll fix it.
>

Ah, I took a closer look. It turns out that when you combine the
"source-none" and
"interactive" classes, the result has too low of a contrast ratio between
the foreground
and background color.

#events-view-source-list-tbody .source-none {
  color: rgb(238, 0, 0);
}

#events-view-source-list-tbody .inactive {
  background: rgb(245, 255, 245);
}

That's the reason rgb(235, 0, 0) was needed here.

- Dominic

[mmenke, 2013-08-05 17:34:45]

On 2013/08/05 17:21:29, Dominic Mazzoni wrote:
> On Mon, Aug 5, 2013 at 9:51 AM, <mailto:mmenke@chromium.org> wrote:
> 
> > It's more the color changes and
> >
> 
> It seems reasonable to relax the requirements a bit for internal pages, and
> only
> warn for text that would be extremely hard to read.
> 
> 
> > ignoreSelectors thing that make me a bit
> > concerned about future maintenance overhead.
> >
> 
> This seems solvable. The audit could actually print out the exact code to
> use, something
> like this:
> 
> # If you want to suppress this error, add this line of code to your setUp
> method:
> this.accessibilityAuditConfig().ignoreSelectors(...
> # If you want to suppress all errors of this type...
> this.accessibilityAuditConfig().ignoreSelectors(...
> # If you want to ignore all accessibility errors for this test:
> this.accessibilityIssuesAreErrors = false;
> 
> - Dominic

The clearer the output from the audit, the better.  That was another concern I
had (That I forgot to mention...).  Automated validation output can be rather
inscrutable.

[mmenke, 2013-08-05 17:35:47]

On 2013/08/05 17:24:02, Dominic Mazzoni wrote:
> On Mon, Aug 5, 2013 at 9:47 AM, <mailto:dmazzoni@chromium.org> wrote:
> 
> > On 2013/08/05 16:42:59, mmenke (incorrect) wrote:
> >
> >> But they're different colors:  #E00 is rgb(238, 0, 0), I believe.  There's
> >>
> > also
> >
> >> one place where you actually used rgb(238, 0, 0), but in most places you
> >> went
> >> with 235 instead.  Not a big deal, but I was just wondering if there's a
> >>
> > reason
> >
> >> for the slightly different shades of red.
> >>
> >
> > You're right. The 235 looks like my mistake. It looks like #e00 / rgb(238,
> > 0, 0)
> > should be used everywhere. I'll fix it.
> >
> 
> Ah, I took a closer look. It turns out that when you combine the
> "source-none" and
> "interactive" classes, the result has too low of a contrast ratio between
> the foreground
> and background color.
> 
> #events-view-source-list-tbody .source-none {
>   color: rgb(238, 0, 0);
> }
> 
> #events-view-source-list-tbody .inactive {
>   background: rgb(245, 255, 245);
> }
> 
> That's the reason rgb(235, 0, 0) was needed here.
> 
> - Dominic

Thanks for the explanation (And still LGTM)

[aboxhall1, 2013-08-05 18:01:53]

Thanks for the feedback - we're working on adding more information and
suggestions to the audit output, and looking at ways to catch false
positives in the audit so that less configuration is required.


On Mon, Aug 5, 2013 at 10:34 AM, <mmenke@chromium.org> wrote:

> On 2013/08/05 17:21:29, Dominic Mazzoni wrote:
>
>  On Mon, Aug 5, 2013 at 9:51 AM, <mailto:mmenke@chromium.org> wrote:
>>
>
>  > It's more the color changes and
>> >
>>
>
>  It seems reasonable to relax the requirements a bit for internal pages,
>> and
>> only
>> warn for text that would be extremely hard to read.
>>
>
>
>  > ignoreSelectors thing that make me a bit
>> > concerned about future maintenance overhead.
>> >
>>
>
>  This seems solvable. The audit could actually print out the exact code to
>> use, something
>> like this:
>>
>
>  # If you want to suppress this error, add this line of code to your setUp
>> method:
>> this.accessibilityAuditConfig(**).ignoreSelectors(...
>> # If you want to suppress all errors of this type...
>> this.accessibilityAuditConfig(**).ignoreSelectors(...
>> # If you want to ignore all accessibility errors for this test:
>> this.**accessibilityIssuesAreErrors = false;
>>
>
>  - Dominic
>>
>
> The clearer the output from the audit, the better.  That was another
> concern I
> had (That I forgot to mention...).  Automated validation output can be
> rather
> inscrutable.
>
>
https://codereview.chromium.**org/21944004/<https://codereview.chromium.org/2...
>

[commit-bot: I haz the power, 2013-08-15 05:51:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/21944004/71002

[commit-bot: I haz the power, 2013-08-15 11:15:15]

Change committed as 217775