MD Settings: add a bunch of dialog (X) aria-labels

MD Settings: add a bunch of dialog (X) aria-labels

R=dpapad@chromium.org
BUG=693373
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2717613002
Cr-Commit-Position: refs/heads/master@{#452744}
Committed: https://chromium.googlesource.com/chromium/src/+/067993e006fab2c08cd42cda616e65e682b17305

[Dan Beam, 2017-02-24 00:07:08]

Description was changed from

==========
MD Settings: add a bunch of dialog (X) aria-labels

R=dpapad@chromium.org
BUG=693373
==========

to

==========
MD Settings: add a bunch of dialog (X) aria-labels

R=dpapad@chromium.org
BUG=693373
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-02-24 00:25:16]

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

[commit-bot: I haz the power, 2017-02-24 00:25:38]

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

[dpapad, 2017-02-24 00:39:06]

On 2017/02/24 at 00:25:10, dbeam wrote:
> 

LGTM, but also summarizing offline conversation: I would be fine with directly
using i18n{close} inside cr-dialog itself, since it is a pretty generic string,
see [1]. This would make the default case less verbose (and we could override it
only in the rare cases where we have to, no such case exists yet).

[1]
https://cs.chromium.org/chromium/src/components/components_strings.grd?type=c...

Also, I was not aware of the changes at
https://codereview.chromium.org/2441043004, so I was wondering how this CL works
without updating cr-dialog. Having said that, the original CL could have added a
test to ensure that the close-text="foo" propagates to the aria-label of the
close button. Up to you if you want to add this test here.

[Dan Beam, 2017-02-24 01:06:16]

The CQ bit was unchecked by dbeam@chromium.org

[Dan Beam, 2017-02-24 01:06:28]

On 2017/02/24 00:39:06, dpapad wrote:
> On 2017/02/24 at 00:25:10, dbeam wrote:
> > 
> 
> LGTM, but also summarizing offline conversation: I would be fine with directly
> using i18n{close} inside cr-dialog itself, since it is a pretty generic
string,
> see [1]. This would make the default case less verbose (and we could override
it
> only in the rare cases where we have to, no such case exists yet).
> 
> [1]
>
https://cs.chromium.org/chromium/src/components/components_strings.grd?type=c...
> 
> Also, I was not aware of the changes at
> https://codereview.chromium.org/2441043004, so I was wondering how this CL
works
> without updating cr-dialog. Having said that, the original CL could have added
a
> test to ensure that the close-text="foo" propagates to the aria-label of the
> close button. Up to you if you want to add this test here.

not really

[Dan Beam, 2017-02-24 01:06:33]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2017-02-24 01:07:03]

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, 2017-02-24 01:53:26]

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

[commit-bot: I haz the power, 2017-02-24 01:53:28]

Failed to apply patch for
chrome/browser/resources/settings/site_settings/add_site_dialog.html:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/resources/settings/site_settings/add_site_dialog.html:17
  error: chrome/browser/resources/settings/site_settings/add_site_dialog.html:
patch does not apply

Patch:      
chrome/browser/resources/settings/site_settings/add_site_dialog.html
Index: chrome/browser/resources/settings/site_settings/add_site_dialog.html
diff --git
a/chrome/browser/resources/settings/site_settings/add_site_dialog.html
b/chrome/browser/resources/settings/site_settings/add_site_dialog.html
index
149cad2ada67ed2e85d6912c465477ba6c761b7d..0eed6e29d565a3d4211e6682bfd1d9c2232c935c
100644
--- a/chrome/browser/resources/settings/site_settings/add_site_dialog.html
+++ b/chrome/browser/resources/settings/site_settings/add_site_dialog.html
@@ -17,7 +17,7 @@
         margin-top: 15px;
       }
     </style>
-    <dialog is="cr-dialog" id="dialog">
+    <dialog is="cr-dialog" id="dialog" close-text="$i18n{close}">
       <div class="title">$i18n{addSiteHeader}</div>
       <div class="body">
         <span class="start">$i18n{addSite}</span>

[Dan Beam, 2017-02-24 02:59:24]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2017-02-24 02:59:25]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2717613002/#ps20001
(title: ".")

[commit-bot: I haz the power, 2017-02-24 02:59:58]

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, 2017-02-24 03:58:36]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487905164815000,
"parent_rev": "fab6dba98d3356e685ffee7ca2cd6acadb2fb4f9", "commit_rev":
"067993e006fab2c08cd42cda616e65e682b17305"}

[commit-bot: I haz the power, 2017-02-24 03:59:16]

Description was changed from

==========
MD Settings: add a bunch of dialog (X) aria-labels

R=dpapad@chromium.org
BUG=693373
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: add a bunch of dialog (X) aria-labels

R=dpapad@chromium.org
BUG=693373
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2717613002
Cr-Commit-Position: refs/heads/master@{#452744}
Committed:
https://chromium.googlesource.com/chromium/src/+/067993e006fab2c08cd42cda616e...
==========

[commit-bot: I haz the power, 2017-02-24 03:59:17]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/067993e006fab2c08cd42cda616e...