Do not fire AXLayoutComplete events based on feedback from Apple

that voiceover does not use them. Bug: 1049320 Change-Id: Ic7059a1c1f2a9800afca5a711994a5e485163564 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2357629Reviewed-by: Aaron Leventhal <aleventhal@chromium.org> Commit-Queue: Abigail Klein <abigailbklein@google.com> Cr-Commit-Position: refs/heads/master@{#798305}

Do not fire AXLayoutComplete events based on feedback from Apple
that voiceover does not use them. Bug: 1049320 Change-Id: Ic7059a1c1f2a9800afca5a711994a5e485163564 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2357629Reviewed-by: Aaron Leventhal <aleventhal@chromium.org> Commit-Queue: Abigail Klein <abigailbklein@google.com> Cr-Commit-Position: refs/heads/master@{#798305}
77c5e475 · Abigail Klein · Commit Bot · f102581d · 77c5e475 · 77c5e475
Commit 77c5e475 authored Aug 14, 2020 by Abigail Klein Committed by Commit Bot Aug 14, 2020
7 changed files
--- a/content/browser/accessibility/browser_accessibility_manager_mac.mm
+++ b/content/browser/accessibility/browser_accessibility_manager_mac.mm
@@ -66,9 +66,9 @@ enum AXTextEditType {
  AXTextEditTypeAttributesChange
 };
+// Native mac notifications fired.
 NSString* const NSAccessibilityAutocorrectionOccurredNotification =
    @"AXAutocorrectionOccurred";
-NSString* const NSAccessibilityLayoutCompleteNotification = @"AXLayoutComplete";
 NSString* const NSAccessibilityNewDocumentLoadCompleteNotification =
    @"AXNewDocumentLoadComplete";
 NSString* const NSAccessibilityInvalidStatusChangedNotification =
@@ -81,6 +81,9 @@ NSString* const NSAccessibilityExpandedChanged = @"AXExpandedChanged";
 NSString* const NSAccessibilityMenuItemSelectedNotification =
    @"AXMenuItemSelected";
+// The following native mac notifications are not fired:
+// AXLayoutComplete: Voiceover does not use this, it is considered too spammy.
 // Attributes used for NSAccessibilitySelectedTextChangedNotification and
 // NSAccessibilityValueChangedNotification.
 NSString* const NSAccessibilityTextStateChangeTypeKey =
@@ -163,9 +166,6 @@ void BrowserAccessibilityManagerMac::FireBlinkEvent(
    case ax::mojom::Event::kAutocorrectionOccured:
      mac_notification = NSAccessibilityAutocorrectionOccurredNotification;
      break;
-    case ax::mojom::Event::kLayoutComplete:
-      mac_notification = NSAccessibilityLayoutCompleteNotification;
-      break;
    default:
      return;
  }
@@ -223,23 +223,18 @@ void BrowserAccessibilityManagerMac::FireGeneratedEvent(
      break;
    case ui::AXEventGenerator::Event::LOAD_COMPLETE:
      // |NSAccessibilityNewDocumentLoadCompleteNotification| should only be
-      // fired on the top document. Iframes should use
+      // fired on the top document.
-      // |ax::mojom::Event::kLayoutComplete| to signify that they have finished
+      // |NSAccessibilityNewDocumentLoadCompleteNotification| is the event that
-      // loading. |NSAccessibilityNewDocumentLoadCompleteNotification| is the
+      // Webkit notifies VoiceOver that a page load has completed.
-      // event that Webkit notifies VoiceOver that a page load has completed.
      // TODO(crbug.com/1049320): Verify in MacOS 10.16 that the "Automatically
      // speak the webpage" option in the VoiceOver utility is triggered upon
      // observing this event.
      if (IsRootTree()) {
        mac_notification = NSAccessibilityNewDocumentLoadCompleteNotification;
      } else {
-        mac_notification = NSAccessibilityLayoutCompleteNotification;
+        return;
      }
      break;
-    case ui::AXEventGenerator::Event::PORTAL_ACTIVATED:
-      DCHECK(IsRootTree());
-      mac_notification = NSAccessibilityLayoutCompleteNotification;
-      break;
    case ui::AXEventGenerator::Event::INVALID_STATUS_CHANGED:
      mac_notification = NSAccessibilityInvalidStatusChangedNotification;
      break;
@@ -445,6 +440,7 @@ void BrowserAccessibilityManagerMac::FireGeneratedEvent(
    case ui::AXEventGenerator::Event::OBJECT_ATTRIBUTE_CHANGED:
    case ui::AXEventGenerator::Event::OTHER_ATTRIBUTE_CHANGED:
    case ui::AXEventGenerator::Event::PLACEHOLDER_CHANGED:
+    case ui::AXEventGenerator::Event::PORTAL_ACTIVATED:
    case ui::AXEventGenerator::Event::POSITION_IN_SET_CHANGED:
    case ui::AXEventGenerator::Event::READONLY_CHANGED:
    case ui::AXEventGenerator::Event::RELATED_NODE_CHANGED:

--- a/content/test/data/accessibility/event/aria-slider-value-both-change.html
+++ b/content/test/data/accessibility/event/aria-slider-value-both-change.html
 <!--
 @WIN-DENY:*
 @WIN-ALLOW:EVENT_OBJECT_VALUECHANGE*
-@MAC-DENY:AXLayoutComplete*
 -->
 <!DOCTYPE html>
 <html>

--- a/content/test/data/accessibility/event/aria-slider-value-change.html
+++ b/content/test/data/accessibility/event/aria-slider-value-change.html
 <!--
 @WIN-DENY:*
 @WIN-ALLOW:EVENT_OBJECT_VALUECHANGE*
-@MAC-DENY:AXLayoutComplete*
 -->
 <!DOCTYPE html>
 <html>

--- a/content/test/data/accessibility/event/aria-slider-valuetext-change.html
+++ b/content/test/data/accessibility/event/aria-slider-valuetext-change.html
 <!--
 @WIN-DENY:*
 @WIN-ALLOW:EVENT_OBJECT_VALUECHANGE*
-@MAC-DENY:AXLayoutComplete*
 -->
 <!DOCTYPE html>
 <html>

--- a/content/test/data/accessibility/event/aria-spinbutton-value-both-change.html
+++ b/content/test/data/accessibility/event/aria-spinbutton-value-both-change.html
@@ -2,7 +2,6 @@
 <!--
 @WIN-DENY:*
 @WIN-ALLOW:EVENT_OBJECT_VALUECHANGE*
-@MAC-DENY:AXLayoutComplete*
 -->
 <html>
 <body>

--- a/content/test/data/accessibility/event/aria-spinbutton-value-change.html
+++ b/content/test/data/accessibility/event/aria-spinbutton-value-change.html
@@ -2,7 +2,6 @@
 <!--
 @WIN-DENY:*
 @WIN-ALLOW:EVENT_OBJECT_VALUECHANGE*
-@MAC-DENY:AXLayoutComplete*
 -->
 <html>
 <body>

--- a/content/test/data/accessibility/event/aria-spinbutton-valuetext-change.html
+++ b/content/test/data/accessibility/event/aria-spinbutton-valuetext-change.html
@@ -5,7 +5,6 @@
 @UIA-WIN-DENY:*
 @UIA-WIN-ALLOW:AriaProperties*
 @UIA-WIN-ALLOW:RangeValueValue*
-@MAC-DENY:AXLayoutComplete*
 -->
 <html>
 <body>