Description was changed from ========== ChromeOS OOBE: More accessibility fixes. This CL updates the following ...
3 years, 9 months ago
(2017-02-28 01:48:40 UTC)
#1
Description was changed from
==========
ChromeOS OOBE: More accessibility fixes.
This CL updates the following code:
1) Ensures aria-labels are DOM-attributes.
2) Updates some labels for OOBE.
3) Fixes focusing and input on EULA screen.
BUG=690725
==========
to
==========
ChromeOS OOBE: More accessibility fixes.
This CL updates the following code:
1) Ensures aria-labels are DOM-attributes.
2) Updates some labels for OOBE.
3) Fixes focusing and input on EULA screen.
BUG=690725
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
Alexander Alekseev
The CQ bit was checked by alemate@chromium.org to run a CQ dry run
3 years, 9 months ago
(2017-02-28 01:49:07 UTC)
#2
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/331168)
3 years, 9 months ago
(2017-03-01 05:30:18 UTC)
#13
3 years, 9 months ago
(2017-03-01 07:43:19 UTC)
#17
Dry run: This issue passed the CQ dry run.
michaelpg
lgtm with nits. the i18n thing is a nice-to-have but not necessary. https://codereview.chromium.org/2718133004/diff/1/chrome/browser/resources/chromeos/login/oobe_buttons.html File chrome/browser/resources/chromeos/login/oobe_buttons.html ...
3 years, 9 months ago
(2017-03-01 20:26:49 UTC)
#18
lgtm with nits. the i18n thing is a nice-to-have but not necessary.
https://codereview.chromium.org/2718133004/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_buttons.html (right):
https://codereview.chromium.org/2718133004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_buttons.html:40: 'label-for-aria' -
accessibility label.
On 2017/03/01 00:48:33, Alexander Alekseev wrote:
> On 2017/02/28 10:38:17, michaelpg wrote:
> > What's the advantage of this renaming? Having the same property name in
oobe-*
> > and gaia-* elements is nice.
>
> Because ChromeVoX will pronounce it.
> We discussed this with Steven in another CL:
>
https://codereview.chromium.org/2688153004/diff/1/chrome/browser/resources/ch...
Thanks for the explanation. It would be helpful to leave a comment (probably in
the .js) explaining why the property is like that: we want to pass the label
value but not actually declare it as an ARIA property here.
https://codereview.chromium.org/2718133004/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_welcome.html (right):
https://codereview.chromium.org/2718133004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:87:
label-for-aria="[[formatMessage_('languageDropdownLabel')]]">
On 2017/03/01 00:48:34, Alexander Alekseev wrote:
> On 2017/02/28 10:38:17, michaelpg wrote:
> > no parameter means no need for formatMessage_, just use i18n-values?
> >
> > if you can do that in this entire file, we can remove formatMessage_ from
> > oobe_welcome.js
>
> I don't know how to use i18n-values for polymer attributes. ($i18n{} would not
> work.) Could you suggest a sample code?
It is explained in i18n_template_no_process.js and used in several places in
OOBE as examples. I don't think Polymer makes it any different, so:
i18n-values="label-for-aria:languageDropdownLabel"
If $i18n{} would work, it would look like:
label-for-aria="$i18n{languageDropdownLabel}"
Either way, you are setting the attribute directly, not binding it to another
Polymer property.
https://codereview.chromium.org/2718133004/diff/40001/chrome/app/chromeos_str...
File chrome/app/chromeos_strings.grdp (right):
https://codereview.chromium.org/2718133004/diff/40001/chrome/app/chromeos_str...
chrome/app/chromeos_strings.grdp:1788: <message
name="IDS_TIMEZONE_DROPDOWN_LABEL" desc="Navigation label attached to the 'List
of timezones' dropdown menu on ChromeOS Timezone initial device setup screen"
meaning="The menu itself displays only current timezone name. But for
accessibility mode we need full description of this menu. So this label suggests
that activating a menu will allow to change current timezone.">
nit: Chrome OS, not ChromeOS
3 years, 9 months ago
(2017-03-01 21:49:09 UTC)
#19
https://codereview.chromium.org/2718133004/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_buttons.html (right):
https://codereview.chromium.org/2718133004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_buttons.html:40: 'label-for-aria' -
accessibility label.
On 2017/03/01 20:26:49, michaelpg wrote:
> On 2017/03/01 00:48:33, Alexander Alekseev wrote:
> > On 2017/02/28 10:38:17, michaelpg wrote:
> > > What's the advantage of this renaming? Having the same property name in
> oobe-*
> > > and gaia-* elements is nice.
> >
> > Because ChromeVoX will pronounce it.
> > We discussed this with Steven in another CL:
> >
>
https://codereview.chromium.org/2688153004/diff/1/chrome/browser/resources/ch...
>
> Thanks for the explanation. It would be helpful to leave a comment (probably
in
> the .js) explaining why the property is like that: we want to pass the label
> value but not actually declare it as an ARIA property here.
Done.
https://codereview.chromium.org/2718133004/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_welcome.html (right):
https://codereview.chromium.org/2718133004/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:87:
label-for-aria="[[formatMessage_('languageDropdownLabel')]]">
On 2017/03/01 20:26:49, michaelpg wrote:
> On 2017/03/01 00:48:34, Alexander Alekseev wrote:
> > On 2017/02/28 10:38:17, michaelpg wrote:
> > > no parameter means no need for formatMessage_, just use i18n-values?
> > >
> > > if you can do that in this entire file, we can remove formatMessage_ from
> > > oobe_welcome.js
> >
> > I don't know how to use i18n-values for polymer attributes. ($i18n{} would
not
> > work.) Could you suggest a sample code?
>
> It is explained in i18n_template_no_process.js and used in several places in
> OOBE as examples. I don't think Polymer makes it any different, so:
> i18n-values="label-for-aria:languageDropdownLabel"
>
> If $i18n{} would work, it would look like:
> label-for-aria="$i18n{languageDropdownLabel}"
>
> Either way, you are setting the attribute directly, not binding it to another
> Polymer property.
Done.
https://codereview.chromium.org/2718133004/diff/40001/chrome/app/chromeos_str...
File chrome/app/chromeos_strings.grdp (right):
https://codereview.chromium.org/2718133004/diff/40001/chrome/app/chromeos_str...
chrome/app/chromeos_strings.grdp:1788: <message
name="IDS_TIMEZONE_DROPDOWN_LABEL" desc="Navigation label attached to the 'List
of timezones' dropdown menu on ChromeOS Timezone initial device setup screen"
meaning="The menu itself displays only current timezone name. But for
accessibility mode we need full description of this menu. So this label suggests
that activating a menu will allow to change current timezone.">
On 2017/03/01 20:26:49, michaelpg wrote:
> nit: Chrome OS, not ChromeOS
Done.
Alexander Alekseev
The CQ bit was checked by alemate@chromium.org
3 years, 9 months ago
(2017-03-01 21:49:18 UTC)
#20
CQ is committing da patch. Bot data: {"patchset_id": 60001, "attempt_start_ts": 1488404958147180, "parent_rev": "c7f36df401d6c79813e1137e4d67e0bb0ecf8557", "commit_rev": "7e2c6819395a5185f0f90d6cff8f3a47c444ce86"}
3 years, 9 months ago
(2017-03-01 22:59:08 UTC)
#23
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1488404958147180,
"parent_rev": "c7f36df401d6c79813e1137e4d67e0bb0ecf8557", "commit_rev":
"7e2c6819395a5185f0f90d6cff8f3a47c444ce86"}
commit-bot: I haz the power
Description was changed from ========== ChromeOS OOBE: More accessibility fixes. This CL updates the following ...
3 years, 9 months ago
(2017-03-01 22:59:44 UTC)
#24
Message was sent while issue was closed.
Description was changed from
==========
ChromeOS OOBE: More accessibility fixes.
This CL updates the following code:
1) Ensures aria-labels are DOM-attributes.
2) Updates some labels for OOBE.
3) Fixes focusing and input on EULA screen.
BUG=690725
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========
to
==========
ChromeOS OOBE: More accessibility fixes.
This CL updates the following code:
1) Ensures aria-labels are DOM-attributes.
2) Updates some labels for OOBE.
3) Fixes focusing and input on EULA screen.
BUG=690725
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2718133004
Cr-Commit-Position: refs/heads/master@{#454082}
Committed:
https://chromium.googlesource.com/chromium/src/+/7e2c6819395a5185f0f90d6cff8f...
==========
commit-bot: I haz the power
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/7e2c6819395a5185f0f90d6cff8f3a47c444ce86
3 years, 9 months ago
(2017-03-01 22:59:45 UTC)
#25
Issue 2718133004: ChromeOS OOBE: More accessibility fixes.
(Closed)
Created 3 years, 9 months ago by Alexander Alekseev
Modified 3 years, 9 months ago
Reviewers: michaelpg
Base URL:
Comments: 16
Chromium Code Reviews
Issue
2718133004:
ChromeOS OOBE: More accessibility fixes. (Closed)
