Correcting issues noted in a11y review for Desktop FRE.

Correcting issues noted in a11y review for Desktop FRE.

BUG=657174, 659759
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/49d714d3f447e3f54372a6a735d18694846a503a
Cr-Commit-Position: refs/heads/master@{#428250}

[tmartino, 2016-10-27 18:59:47]

Description was changed from

==========
Correcting issues noted in a11y review for Desktop FRE.

BUG=657174, 659759
==========

to

==========
Correcting issues noted in a11y review for Desktop FRE.

BUG=657174, 659759
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tmartino, 2016-10-27 19:00:44]

tmartino@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2016-10-27 19:16:07]

https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
File chrome/browser/resources/welcome/welcome.css (right):

https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/welcome.css:123: font-weight: 500;
you probably don't need the document.fonts.load() line if you show bold text by
default, fyi

https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
File chrome/browser/resources/welcome/welcome.html (left):

https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/welcome.html:18: <div class="logo-icon"></div>
i think it's more useful to hide this from screenreaders than try to describe
it.

can we just use role="presentation" (maybe?) or aria-hidden="true" in the
original markup?

https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
File chrome/browser/resources/welcome/welcome.html (right):

https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/welcome.html:19:
srcset="chrome://welcome/logo.png 1x, chrome://welcome/logo2x.png 2x">
80 col wrap

[tmartino, 2016-10-27 19:40:37]

On 2016/10/27 at 19:16:07, dbeam wrote:
>
https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
> File chrome/browser/resources/welcome/welcome.css (right):
> 
>
https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
> chrome/browser/resources/welcome/welcome.css:123: font-weight: 500;
> you probably don't need the document.fonts.load() line if you show bold text
by default, fyi
> 
>
https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
> File chrome/browser/resources/welcome/welcome.html (left):
> 
>
https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
> chrome/browser/resources/welcome/welcome.html:18: <div
class="logo-icon"></div>
> i think it's more useful to hide this from screenreaders than try to describe
it.
> 
> can we just use role="presentation" (maybe?) or aria-hidden="true" in the
original markup?
> 
>
https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
> File chrome/browser/resources/welcome/welcome.html (right):
> 
>
https://codereview.chromium.org/2454173002/diff/1/chrome/browser/resources/we...
> chrome/browser/resources/welcome/welcome.html:19:
srcset="chrome://welcome/logo.png 1x, chrome://welcome/logo2x.png 2x">
> 80 col wrap

PTAL

[Dan Beam, 2016-10-27 21:20:25]

https://codereview.chromium.org/2454173002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/welcome/welcome.css (right):

https://codereview.chromium.org/2454173002/diff/20001/chrome/browser/resource...
chrome/browser/resources/welcome/welcome.css:138: background:
-webkit-image-set(url(chrome://welcome/logo.png) 1x,
nit: background-image

https://codereview.chromium.org/2454173002/diff/20001/chrome/browser/resource...
chrome/browser/resources/welcome/welcome.css:140: background-position: center;
is this needed?  if yes: we should probably have a background-repeat: no-repeat
somewhere as well

https://codereview.chromium.org/2454173002/diff/20001/chrome/browser/resource...
chrome/browser/resources/welcome/welcome.css:141: background-size: 96px 96px;
can "96px 96px" just be 100%?

[tmartino, 2016-10-27 23:14:11]

On 2016/10/27 at 21:20:25, dbeam wrote:
>
https://codereview.chromium.org/2454173002/diff/20001/chrome/browser/resource...
> File chrome/browser/resources/welcome/welcome.css (right):
> 
>
https://codereview.chromium.org/2454173002/diff/20001/chrome/browser/resource...
> chrome/browser/resources/welcome/welcome.css:138: background:
-webkit-image-set(url(chrome://welcome/logo.png) 1x,
> nit: background-image
> 
>
https://codereview.chromium.org/2454173002/diff/20001/chrome/browser/resource...
> chrome/browser/resources/welcome/welcome.css:140: background-position: center;
> is this needed?  if yes: we should probably have a background-repeat:
no-repeat somewhere as well
> 
>
https://codereview.chromium.org/2454173002/diff/20001/chrome/browser/resource...
> chrome/browser/resources/welcome/welcome.css:141: background-size: 96px 96px;
> can "96px 96px" just be 100%?

Done to all.

[Dan Beam, 2016-10-27 23:25:41]

lgtm

[tmartino, 2016-10-28 00:00:35]

The CQ bit was checked by tmartino@chromium.org

[commit-bot: I haz the power, 2016-10-28 00:01:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 02:20:34]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-28 02:24:29]

Description was changed from

==========
Correcting issues noted in a11y review for Desktop FRE.

BUG=657174, 659759
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Correcting issues noted in a11y review for Desktop FRE.

BUG=657174, 659759
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/49d714d3f447e3f54372a6a735d18694846a503a
Cr-Commit-Position: refs/heads/master@{#428250}
==========

[commit-bot: I haz the power, 2016-10-28 02:24:30]

Patchset 3 (id:??) landed as
https://crrev.com/49d714d3f447e3f54372a6a735d18694846a503a
Cr-Commit-Position: refs/heads/master@{#428250}