Android Accessibility: children of an aria-hidden iframe aren't interesting.

To repro, create a page with: <iframe src="..." aria-hidden="true"></iframe> When swiping left/right, TalkBack shouldn't navigate to any elements inside the iframe. The fix is to mark those nodes as not "interesting" internally, that means we won't navigate to those elements. Bug: 1046491 Change-Id: I48ba337829035cc6cd1508df8e78f6d460e36536 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023482Reviewed-by: Akihiro Ota <akihiroota@chromium.org> Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org> Cr-Commit-Position: refs/heads/master@{#736919}

Android Accessibility: children of an aria-hidden iframe aren't interesting.
To repro, create a page with: <iframe src="..." aria-hidden="true"></iframe> When swiping left/right, TalkBack shouldn't navigate to any elements inside the iframe. The fix is to mark those nodes as not "interesting" internally, that means we won't navigate to those elements. Bug: 1046491 Change-Id: I48ba337829035cc6cd1508df8e78f6d460e36536 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023482Reviewed-by: Akihiro Ota <akihiroota@chromium.org> Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org> Cr-Commit-Position: refs/heads/master@{#736919}
46c81871 · Dominic Mazzoni · Commit Bot · b8702bb8 · 46c81871 · 46c81871
Commit 46c81871 authored Jan 30, 2020 by Dominic Mazzoni Committed by Commit Bot Jan 30, 2020
6 changed files
--- a/content/browser/accessibility/browser_accessibility_android.cc
+++ b/content/browser/accessibility/browser_accessibility_android.cc
@@ -348,7 +348,22 @@ bool BrowserAccessibilityAndroid::IsInterestingOnAndroid() const {
  if (HasState(ax::mojom::State::kInvisible))
    return false;

-  // Focusable nodes are always interesting. Note that IsFocusable()
+  // Walk up the ancestry. A non-focusable child of a control is not
+  // interesting. A child of an invisible iframe is also not interesting.
+  const BrowserAccessibility* parent = PlatformGetParent();
+  while (parent != nullptr) {
+    if (ui::IsControl(parent->GetRole()) && !IsFocusable())
+      return false;
+
+    if (parent->GetRole() == ax::mojom::Role::kIframe &&
+        parent->GetData().HasState(ax::mojom::State::kInvisible)) {
+      return false;
+    }
+
+    parent = parent->PlatformGetParent();
+  }
+
+  // Otherwise, focusable nodes are always interesting. Note that IsFocusable()
  // already skips over things like iframes and child frames that are
  // technically focusable but shouldn't be exposed as focusable on Android.
  if (IsFocusable())
@@ -358,14 +373,6 @@ bool BrowserAccessibilityAndroid::IsInterestingOnAndroid() const {
  if (ui::IsControl(GetRole()))
    return true;

-  // A non focusable child of a control is not interesting
-  const BrowserAccessibility* parent = PlatformGetParent();
-  while (parent != nullptr) {
-    if (ui::IsControl(parent->GetRole()))
-      return false;
-    parent = parent->PlatformGetParent();
-  }
-
  // Otherwise, the interesting nodes are leaf nodes with non-whitespace text.
  return PlatformIsLeaf() &&
         !base::ContainsOnlyChars(GetInnerText(), base::kWhitespaceUTF16);

--- a/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
+++ b/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
@@ -646,6 +646,11 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
  RunAriaTest(FILE_PATH_LITERAL("aria-hidden-iframe-body.html"));
 }

+IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
+                       AccessibilityAriaHiddenIframe) {
+  RunAriaTest(FILE_PATH_LITERAL("aria-hidden-iframe.html"));
+}
+
 IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
                       MAYBE(AccessibilityAriaFlowto)) {
  RunAriaTest(FILE_PATH_LITERAL("aria-flowto.html"));

--- a/content/test/data/accessibility/aria/aria-hidden-iframe-expected-android.txt
+++ b/content/test/data/accessibility/aria/aria-hidden-iframe-expected-android.txt
+android.webkit.WebView focusable focused scrollable
++android.view.View
++++android.widget.Button role_description='button' clickable focusable interesting name='Before'
++++android.view.View invisible
++++++android.view.View scrollable
++++++++android.view.View
++++++++++android.widget.Button role_description='button' clickable focusable name='Inner'
++++android.widget.Button role_description='button' clickable focusable interesting name='After'
--- a/content/test/data/accessibility/aria/aria-hidden-iframe-expected-blink.txt
+++ b/content/test/data/accessibility/aria/aria-hidden-iframe-expected-blink.txt
+rootWebArea focusable
++genericContainer
++++button focusable name='Before'
++++++staticText name='Before'
++++++++inlineTextBox name='Before'
++++iframe focusable invisible
++++++rootWebArea focusable
++++++++genericContainer
++++++++++button focusable name='Inner'
++++++++++++staticText name='Inner'
++++++++++++++inlineTextBox name='Inner'
++++button focusable name='After'
++++++staticText name='After'
++++++++inlineTextBox name='After'
\ No newline at end of file
--- a/content/test/data/accessibility/aria/aria-hidden-iframe.html
+++ b/content/test/data/accessibility/aria/aria-hidden-iframe.html
+<!DOCTYPE html>
+<!--
+@BLINK-ALLOW:focusable
+@ANDROID-ALLOW:interesting
+-->
+<button>Before</button>
+<iframe aria-hidden="true" src="frames/inner.html"></iframe>
+<button>After</button>
--- a/content/test/data/accessibility/aria/frames/inner.html
+++ b/content/test/data/accessibility/aria/frames/inner.html
+<button>Inner</button>