Views: Modified all bubbles to have an alert dialog role and deleted accessible name

This small patch fixes a few related bugs.
1. Assigned an alert dialog role for all bubbles.
An alert dialog role makes the contents of a dialog be announced by a screen reader when the dialog appears.
Bubbles are non-modal and thus should be announced as soon as they appear because otherwise the user will not know if they are there.
However, some Windows screen readers do not yet properly support the alert dialog role.
For Windows only, we can switch to the system alert role temporarily until screen reader vendors provide a fix.
2. ARIA Spec compliant?
The ARIA Spec dictates that an alert dialog role should only be used for modal dialogs.
However, there is no other spec-compliant way to announce the contents of a dialog.
3. Deleted accessible name from any dialogs.
The ARIA Spec also dictates that alert dialogs should have an accessible name.
However, this patch modifies Views. In Views all dialogs have a title bar.
Having both an accessible name and a title bar makes Windows screen readers announce the title twice.
4. Applied to all bubbles by default.
Since most bubbles are non-modal and need to be announced as soon as they appear, alert dialog should become the default role.
Subclasses can override if needed and switch back to a dialog role.
Having a role of dialog doesn't announce anything on Windows when the dialog is non-modal.

R=dtseng@chromium.org, aleventhal@chromium.org

Bug: 775705, 779297, 474622
Change-Id: I062110757cd4c400b31aa3cb89c622002699408e
Tested: Jaws and NVDA with permission, password save and zoom bubbles.
Reviewed-on: https://chromium-review.googlesource.com/967465
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546891}

chrome/app/generated_resources.grd

@@ -232,9 +232,6 @@ are declared in build/common.gypi.
       <message name="IDS_PERMISSIONS_BUBBLE_PROMPT" desc="The label that is used to introduce permission request details to the user in a popup.">
         <ph name="SITE_NAME">$1<ex>google.com</ex></ph> wants to
       </message>
-      <message name="IDS_PERMISSIONS_BUBBLE_ACCESSIBLE_TITLE" desc="The sentence read out by screen readers to describe the permission request popup.">
-        <ph name="SITE_NAME">$1<ex>google.com</ex></ph> has a permission request.
-      </message>
       <message name="IDS_PERMISSION_ALLOW" desc="Label on button to allow a permissions request.">
         Allow
       </message>

chrome/browser/ui/views/conflicting_module_view_win.cc

@@ -20,7 +20,6 @@
 #include "chrome/grit/theme_resources.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/notification_service.h"
-#include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/geometry/insets.h"
@@ -106,10 +105,6 @@ void ConflictingModuleView::ShowBubble() {
   bubble_shown.SetValue(bubble_shown.GetValue() + 1);
 }
 
-ax::mojom::Role ConflictingModuleView::GetAccessibleWindowRole() const {
-  return ax::mojom::Role::kAlertDialog;
-}
-
 void ConflictingModuleView::OnWidgetClosing(views::Widget* widget) {
   views::BubbleDialogDelegateView::OnWidgetClosing(widget);
   base::RecordAction(

chrome/browser/ui/views/conflicting_module_view_win.h

@@ -32,8 +32,6 @@ class ConflictingModuleView : public views::BubbleDialogDelegateView,
   // Shows the bubble and updates the counter for how often it has been shown.
   void ShowBubble();
 
-  // views::BubbleDialogDelegateView:
-  ax::mojom::Role GetAccessibleWindowRole() const override;
   void OnWidgetClosing(views::Widget* widget) override;
   bool Accept() override;
   base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;

chrome/browser/ui/views/extensions/extension_message_bubble_view_browsertest.cc

@@ -134,7 +134,7 @@ void ExtensionMessageBubbleViewBrowserTest::ClickLearnMoreButton(
   // to report a valid state.
   ui::AXNodeData node_data;
   bubble->GetWidget()->GetRootView()->GetAccessibleNodeData(&node_data);
-  EXPECT_EQ(ax::mojom::Role::kDialog, node_data.role);
+  EXPECT_EQ(ax::mojom::Role::kAlertDialog, node_data.role);
 }
 
 void ExtensionMessageBubbleViewBrowserTest::ClickActionButton(

chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc

@@ -73,9 +73,7 @@ class PermissionsBubbleDialogDelegateView
 
   void CloseBubble();
 
-  // BubbleDialogDelegateView:
-  ax::mojom::Role GetAccessibleWindowRole() const override;
-  base::string16 GetAccessibleWindowTitle() const override;
+  // BubbleDialogDelegateView overrides.
   bool ShouldShowCloseButton() const override;
   base::string16 GetWindowTitle() const override;
   void OnWidgetDestroying(views::Widget* widget) override;
@@ -178,17 +176,6 @@ void PermissionsBubbleDialogDelegateView::AddedToWidget() {
   GetBubbleFrameView()->SetTitleView(std::move(title));
 }
 
-ax::mojom::Role PermissionsBubbleDialogDelegateView::GetAccessibleWindowRole()
-    const {
-  return ax::mojom::Role::kAlertDialog;
-}
-
-base::string16 PermissionsBubbleDialogDelegateView::GetAccessibleWindowTitle()
-    const {
-  return l10n_util::GetStringFUTF16(IDS_PERMISSIONS_BUBBLE_ACCESSIBLE_TITLE,
-                                    name_or_origin_.name_or_origin);
-}
-
 bool PermissionsBubbleDialogDelegateView::ShouldShowCloseButton() const {
   return true;
 }

content/browser/accessibility/dump_accessibility_tree_browsertest.cc

@@ -358,10 +358,17 @@ IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityAriaAlert) {
   RunAriaTest(FILE_PATH_LITERAL("aria-alert.html"));
 }
 
+#if defined(OS_WIN)
+IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest,
+                       DISABLED_AccessibilityAriaAlertDialog) {
+  RunAriaTest(FILE_PATH_LITERAL("aria-alertdialog.html"));
+}
+#else
 IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest,
                        AccessibilityAriaAlertDialog) {
   RunAriaTest(FILE_PATH_LITERAL("aria-alertdialog.html"));
 }
+#endif
 
 IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest,
                        AccessibilityAriaApplication) {

ui/accessibility/platform/ax_platform_node_win.cc

@@ -2419,7 +2419,10 @@ int AXPlatformNodeWin::MSAARole() {
       return ROLE_SYSTEM_ALERT;
 
     case ax::mojom::Role::kAlertDialog:
-      return ROLE_SYSTEM_DIALOG;
+      // We temporarily use |ROLE_SYSTEM_ALERT| because some Windows screen
+      // readers are not compatible with |ax::mojom::Role::kAlertDialog| yet.
+      // TODO(aleventhal) modify this to return |ROLE_SYSTEM_DIALOG|.
+      return ROLE_SYSTEM_ALERT;
 
     case ax::mojom::Role::kAnchor:
       return ROLE_SYSTEM_LINK;

ui/views/bubble/bubble_dialog_delegate.cc

@@ -250,6 +250,13 @@ gfx::Rect BubbleDialogDelegateView::GetBubbleBounds() {
       adjust_if_offscreen_ && !anchor_minimized && has_anchor);
 }
 
+ax::mojom::Role BubbleDialogDelegateView::GetAccessibleWindowRole() const {
+  // We return |ax::mojom::Role::kAlertDialog| which will make screen
+  // readers announce the contents of the bubble dialog as soon as it appears,
+  // as long as we also fire |ax::mojom::Event::kAlert|.
+  return ax::mojom::Role::kAlertDialog;
+}
+
 void BubbleDialogDelegateView::OnNativeThemeChanged(
     const ui::NativeTheme* theme) {
   UpdateColorsFromTheme(theme);
@@ -325,9 +332,11 @@ void BubbleDialogDelegateView::HandleVisibilityChanged(Widget* widget,
   // the bubble in its entirety rather than just its title and initially focused
   // view.  See http://crbug.com/474622 for details.
   if (widget == GetWidget() && visible) {
-    if (GetAccessibleWindowRole() == ax::mojom::Role::kAlertDialog)
+    if (GetAccessibleWindowRole() == ax::mojom::Role::kAlert ||
+        GetAccessibleWindowRole() == ax::mojom::Role::kAlertDialog) {
       widget->GetRootView()->NotifyAccessibilityEvent(ax::mojom::Event::kAlert,
                                                       true);
+    }
   }
 }
 

ui/views/bubble/bubble_dialog_delegate.h

@@ -10,6 +10,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "build/build_config.h"
+#include "ui/accessibility/ax_enums.mojom.h"
 #include "ui/views/bubble/bubble_border.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_observer.h"
@@ -132,6 +133,9 @@ class VIEWS_EXPORT BubbleDialogDelegateView : public DialogDelegateView,
   // Get bubble bounds from the anchor rect and client view's preferred size.
   virtual gfx::Rect GetBubbleBounds();
 
+  // DialogDelegateView overrides:
+  ax::mojom::Role GetAccessibleWindowRole() const override;
+
   // View overrides:
   void OnNativeThemeChanged(const ui::NativeTheme* theme) override;
 

ui/views/bubble/tray_bubble_view.cc

@@ -313,6 +313,13 @@ int TrayBubbleView::GetDialogButtons() const {
   return ui::DIALOG_BUTTON_NONE;
 }
 
+ax::mojom::Role TrayBubbleView::GetAccessibleWindowRole() const {
+  // We override the role because the base class sets it to alert dialog.
+  // This would make screen readers announce the whole of the system tray
+  // which is undesirable.
+  return ax::mojom::Role::kDialog;
+}
+
 void TrayBubbleView::SizeToContents() {
   BubbleDialogDelegateView::SizeToContents();
   bubble_content_mask_->layer()->SetBounds(GetBubbleBounds());

ui/views/bubble/tray_bubble_view.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "base/optional.h"
+#include "ui/accessibility/ax_enums.mojom.h"
 #include "ui/events/event.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/views/bubble/bubble_dialog_delegate.h"
@@ -160,6 +161,7 @@ class VIEWS_EXPORT TrayBubbleView : public BubbleDialogDelegateView,
  protected:
   // Overridden from views::BubbleDialogDelegateView.
   int GetDialogButtons() const override;
+  ax::mojom::Role GetAccessibleWindowRole() const override;
   void SizeToContents() override;
 
   // Overridden from views::View.