WebUI: Fix toasts not reading out content

Messaging in cr-toasts were not being read out consistently, especially
in cases where the toast text stays the same for multiple actions (for
example, "Bookmark removed." for multiple different bookmarks).

This CL updates the cr-toast's show method to remove the role="alert"
attribute and re-add it to better ensure screen readers recognize to
read out the cr-toast's contents, and fixes one use of cr-toast to use
the show method instead of data-binding a property as the open
attribute.

Using a method is better than using an attribute because each call to
the method is a signal to read out the cr-toast, while using an
attribute will not trigger for any toasts that are shown while a toast
is already visible. For example, setting |open| to true while it is
already set to true will not trigger any JS, but calling |show()| is
guaranteed to always trigger the method's contents.

The plan is to replace all uses of the open attribute to the show method
for better consistency, and to eventually change the open property on
cr-toast to read-only.

Previous params for show were not being used.

Fixed: 1055842
Change-Id: I6c545814a4fc4b465b35708abbfa746c4acbb69a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2477663
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818616}

chrome/browser/resources/settings/privacy_page/personalization_options.html

@@ -109,7 +109,7 @@
     </template>
 
 <if expr="not chromeos">
-    <cr-toast id="toast" open="[[showRestartToast_]]">
+    <cr-toast id="toast">
       <div>$i18n{restartToApplyChanges}</div>
       <cr-button on-click="onRestartTap_">$i18n{restart}</cr-button>
     </cr-toast>

chrome/browser/resources/settings/privacy_page/personalization_options.js

@@ -49,9 +49,6 @@ Polymer({
 
     /** @private */
     showRestart_: Boolean,
-
-    /** @private */
-    showRestartToast_: Boolean,
     // </if>
 
     /** @private */
@@ -192,7 +189,7 @@ Polymer({
       /** @type {!SettingsToggleButtonElement} */ (
           this.$$('#signinAllowedToggle'))
           .sendPrefChange();
-      this.showRestartToast_ = true;
+      this.$.toast.show();
     }
   },
 
@@ -205,7 +202,7 @@ Polymer({
       /** @type {!SettingsToggleButtonElement} */ (
           this.$$('#signinAllowedToggle'))
           .sendPrefChange();
-      this.showRestartToast_ = true;
+      this.$.toast.show();
     }
     this.showSignoutDialog_ = false;
   },

chrome/test/data/webui/cr_elements/cr_toast_test.js

@@ -128,23 +128,4 @@ suite('cr-toast', function() {
     mockTimer.tick(1);
     assertFalse(toast.open);
   });
-
-  test('setting duration using show(duration)', function() {
-    const duration = 100;
-    toast.show(duration);
-    assertTrue(toast.open);
-
-    mockTimer.tick(duration);
-    assertFalse(toast.open);
-  });
-
-  test('setting text', function() {
-    const duration = 100;
-    const text = 'foo';
-    assertEquals('', toast.textContent);
-    toast.show(undefined, text);
-    assertEquals(text, toast.textContent);
-    toast.show(duration);
-    assertEquals(text, toast.textContent);
-  });
 });

chrome/test/data/webui/settings/personalization_options_test.js

@@ -98,8 +98,7 @@ suite('PersonalizationOptionsTests_AllBuilds', function() {
       assertTrue(testElement.$.toast.open);
 
       // Reset toast.
-      testElement.showRestartToast_ = false;
-      assertFalse(testElement.$.toast.open);
+      testElement.$.toast.hide();
 
       // When the user is part way through sync setup, the toggle should be
       // disabled in an on state.

ui/webui/resources/cr_elements/cr_toast/BUILD.gn

@@ -13,12 +13,12 @@ js_type_check("closure_compile") {
 }
 
 js_library("cr_toast") {
-  deps = [ "//third_party/polymer/v1_0/components-chromium/iron-a11y-announcer:iron-a11y-announcer-extracted" ]
 }
 
 js_library("cr_toast_manager") {
   deps = [
     ":cr_toast",
+    "//third_party/polymer/v1_0/components-chromium/iron-a11y-announcer:iron-a11y-announcer-extracted",
     "//ui/webui/resources/js:assert",
     "//ui/webui/resources/js:cr",
   ]

ui/webui/resources/cr_elements/cr_toast/cr_toast.html

 <link rel="import" href="../../html/polymer.html">
 
-<link rel="import" href="chrome://resources/polymer/v1_0/iron-a11y-announcer/iron-a11y-announcer.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html">
 <link rel="import" href="../shared_vars_css.html">
 

ui/webui/resources/cr_elements/cr_toast/cr_toast.js

@@ -20,10 +20,6 @@ Polymer({
     },
   },
 
-  hostAttributes: {
-    'role': 'alert',
-  },
-
   observers: ['resetAutoHide_(duration, open)'],
 
   /** @private {number|null} */
@@ -46,69 +42,31 @@ Polymer({
     }
   },
 
-  /**
-   * Announce a11y message
-   * @param {string} text
-   * @private
-   */
-  announceA11yMessage_(text) {
-    Polymer.IronA11yAnnouncer.requestAvailability();
-    this.fire('iron-announce', { text });
-  },
-
   /**
    * Shows the toast and auto-hides after |this.duration| milliseconds has
    * passed. If the toast is currently being shown, any preexisting auto-hide
    * is cancelled and replaced with a new auto-hide.
-   *
-   * If |this.duration| is set to 0, the toast will remain open indefinitely.
-   * The caller is responsible for hiding the toast.
-   *
-   * When |duration| is passed in the non-negative number, |this.duration|
-   * is updated to that value.
-   *
-   * If text is set, replace the toast content with text,
-   * can also optionally announce a11y with text
-   *
-   * @param {number=} duration
-   * @param {string=} text
-   * @param {boolean=} shouldAnnounceA11y
    */
-  show(duration, text, shouldAnnounceA11y) {
-    if (text !== undefined) {
-      this.textContent = '';
-      const span = document.createElement('span');
-      span.textContent = text;
-      this.appendChild(span);
+  show() {
+    // Force autohide to reset if calling show on an already shown toast.
+    const shouldResetAutohide = this.open;
 
-      if (shouldAnnounceA11y) {
-        this.announceA11yMessage_(text);
-      }
-    }
-
-    // |this.resetAutoHide_| is called whenever |this.duration| or |this.open|
-    // is changed. If neither is changed, we will still need to reset auto-hide.
-    let shouldResetAutoHide = true;
+    // The role attribute is removed first so that screen readers to better
+    // ensure that screen readers will read out the content inside the toast.
+    // If the role is not removed and re-added back in, certain screen readers
+    // do not read out the contents, especially if the text remains exactly
+    // the same as a previous toast.
+    this.removeAttribute('role');
 
-    if (typeof (duration) !== 'undefined' && duration >= 0 &&
-        this.duration !== duration) {
-      this.duration = duration;
-      shouldResetAutoHide = false;
-    }
+    this.open = true;
+    this.setAttribute('role', 'alert');
 
-    if (!this.open) {
-      this.open = true;
-      shouldResetAutoHide = false;
-    }
-
-    if (shouldResetAutoHide) {
+    if (shouldResetAutohide) {
       this.resetAutoHide_();
     }
   },
 
-  /**
-   * Hides the toast.
-   */
+  /** Hides the toast. */
   hide() {
     this.open = false;
   },