Media Controls a11y: add text/description/role to more controls components.

Media Controls a11y: add text/description/role to more controls components.

BUG=516346, 650444
R=dmazzoni@chromium.org

Committed: https://crrev.com/6183e0ce6dcfee2b26f0e07542a349353e13f8f4
Cr-Commit-Position: refs/heads/master@{#424586}

[mlamouri (slow - plz ping), 2016-10-09 18:00:15]

The CQ bit was checked by mlamouri@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-09 18:00:21]

Dry run: 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-09 18:12:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-09 18:12:15]

Dry run: Exceeded global retry quota

[mlamouri (slow - plz ping), 2016-10-09 20:54:14]

The CQ bit was checked by mlamouri@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-09 20:54:18]

Dry run: 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-09 21:04:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-09 21:04:27]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[mlamouri (slow - plz ping), 2016-10-09 22:12:37]

The CQ bit was checked by mlamouri@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-09 22:12:47]

Dry run: 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-09 23:34:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-09 23:34:12]

Dry run: This issue passed the CQ dry run.

[dmazzoni, 2016-10-10 03:39:52]

lgtm with nits/suggestions

https://codereview.chromium.org/2402263002/diff/40001/content/app/strings/con...
File content/app/strings/content_strings.grd (right):

https://codereview.chromium.org/2402263002/diff/40001/content/app/strings/con...
content/app/strings/content_strings.grd:624: download file
Maybe download media?

https://codereview.chromium.org/2402263002/diff/40001/content/app/strings/con...
content/app/strings/content_strings.grd:628: show more controls
How about "more media controls"?

Saying "Show more" implies it just toggles showing more, but really this is just
a menu button

https://codereview.chromium.org/2402263002/diff/40001/content/app/strings/con...
content/app/strings/content_strings.grd:704: download file
This isn't necessary if the text is identical to the button title. The purpose
of the help string is if the button title is short and concise and a longer
description might help clarify what happens when you click on it. In this case I
think just skipping it is better.

https://codereview.chromium.org/2402263002/diff/40001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebLocalizedString.h (right):

https://codereview.chromium.org/2402263002/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebLocalizedString.h:63:
AxMediaDownloadButton,
Capitalization doesn't seem to match the rest of the list

[mlamouri (slow - plz ping), 2016-10-10 04:27:07]

The CQ bit was checked by mlamouri@chromium.org to run a CQ dry run

[mlamouri (slow - plz ping), 2016-10-10 04:27:13]

https://codereview.chromium.org/2402263002/diff/40001/content/app/strings/con...
File content/app/strings/content_strings.grd (right):

https://codereview.chromium.org/2402263002/diff/40001/content/app/strings/con...
content/app/strings/content_strings.grd:624: download file
On 2016/10/10 at 03:39:52, dmazzoni wrote:
> Maybe download media?

Done. I'm not entirely sure people really understand "download media" compared
to "download file" but I'm no native speaker so :)

https://codereview.chromium.org/2402263002/diff/40001/content/app/strings/con...
content/app/strings/content_strings.grd:628: show more controls
On 2016/10/10 at 03:39:52, dmazzoni wrote:
> How about "more media controls"?
> 
> Saying "Show more" implies it just toggles showing more, but really this is
just a menu button

Done.

https://codereview.chromium.org/2402263002/diff/40001/content/app/strings/con...
content/app/strings/content_strings.grd:704: download file
On 2016/10/10 at 03:39:52, dmazzoni wrote:
> This isn't necessary if the text is identical to the button title. The purpose
of the help string is if the button title is short and concise and a longer
description might help clarify what happens when you click on it. In this case I
think just skipping it is better.

Done. FTR, a lot of "help" description seems to be exactly like the alt text.

https://codereview.chromium.org/2402263002/diff/40001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebLocalizedString.h (right):

https://codereview.chromium.org/2402263002/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebLocalizedString.h:63:
AxMediaDownloadButton,
On 2016/10/10 at 03:39:52, dmazzoni wrote:
> Capitalization doesn't seem to match the rest of the list

Done. I fixed some others that were not matching too.

[commit-bot: I haz the power, 2016-10-10 04:27:20]

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

[mlamouri (slow - plz ping), 2016-10-10 04:28:37]

mlamouri@chromium.org changed reviewers:
+ jochen@chromium.org

[mlamouri (slow - plz ping), 2016-10-10 04:28:38]

+jochen@ for rubberstamping boilerplate

[dmazzoni, 2016-10-10 04:45:38]

>
> On 2016/10/10 at 03:39:52, dmazzoni wrote:
> > This isn't necessary if the text is identical to the button title. The
> purpose of the help string is if the button title is short and concise
> and a longer description might help clarify what happens when you click
> on it. In this case I think just skipping it is better.
>
> Done. FTR, a lot of "help" description seems to be exactly like the alt
> text.
>

Acknowledged. I'll remove the redundancy in a future cleanup.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmazzoni, 2016-10-10 04:45:38]

>
> On 2016/10/10 at 03:39:52, dmazzoni wrote:
> > This isn't necessary if the text is identical to the button title. The
> purpose of the help string is if the button title is short and concise
> and a longer description might help clarify what happens when you click
> on it. In this case I think just skipping it is better.
>
> Done. FTR, a lot of "help" description seems to be exactly like the alt
> text.
>

Acknowledged. I'll remove the redundancy in a future cleanup.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2016-10-10 05:50:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-10 05:50:24]

Dry run: This issue passed the CQ dry run.

[mlamouri (slow - plz ping), 2016-10-11 19:55:15]

mlamouri@chromium.org changed reviewers:
+ nasko@chromium.org, pdr@chromium.org
- jochen@chromium.org

[mlamouri (slow - plz ping), 2016-10-11 19:55:16]

-jochen@

+pdr@ for public/platform/
+nasko@ for content/

(this is mostly rubberstamping mechanical changes)

[pdr., 2016-10-11 20:31:43]

On 2016/10/11 at 19:55:16, mlamouri wrote:
> -jochen@
> 
> +pdr@ for public/platform/
 LGTM

[nasko, 2016-10-11 21:51:07]

LGTM

[mlamouri (slow - plz ping), 2016-10-11 21:51:42]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2016-10-11 21:51:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/2402263002/#ps60001
(title: "review comments")

[commit-bot: I haz the power, 2016-10-11 21:52:19]

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-11 23:16:14]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-11 23:18:02]

Description was changed from

==========
Media Controls a11y: add text/description/role to more controls components.

BUG=516346,650444
R=dmazzoni@chromium.org
==========

to

==========
Media Controls a11y: add text/description/role to more controls components.

BUG=516346,650444
R=dmazzoni@chromium.org

Committed: https://crrev.com/6183e0ce6dcfee2b26f0e07542a349353e13f8f4
Cr-Commit-Position: refs/heads/master@{#424586}
==========

[commit-bot: I haz the power, 2016-10-11 23:18:03]

Patchset 4 (id:??) landed as
https://crrev.com/6183e0ce6dcfee2b26f0e07542a349353e13f8f4
Cr-Commit-Position: refs/heads/master@{#424586}