From 7d0eaa6c0c923dfd435bac574c8deaadad0a48bc Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Mon, 7 Aug 2023 11:30:32 -0700 Subject: [PATCH 1/8] Android a11y bridge sets importantness --- .../io/flutter/view/AccessibilityBridge.java | 35 ++++- .../flutter/view/AccessibilityBridgeTest.java | 125 ++++++++++++++++++ 2 files changed, 159 insertions(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/view/AccessibilityBridge.java b/shell/platform/android/io/flutter/view/AccessibilityBridge.java index 134c9fbcff43a..f7e05d59f28ec 100644 --- a/shell/platform/android/io/flutter/view/AccessibilityBridge.java +++ b/shell/platform/android/io/flutter/view/AccessibilityBridge.java @@ -40,6 +40,8 @@ import io.flutter.plugin.platform.PlatformViewsAccessibilityDelegate; import io.flutter.util.Predicate; import io.flutter.util.ViewUtils; +import io.flutter.view.AccessibilityBridge.Flag; + import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.Charset; @@ -575,6 +577,11 @@ private void setBoldTextFlag() { sendLatestAccessibilityFlagsToFlutter(); } + @VisibleForTesting + public AccessibilityNodeInfo obtainAccessibilityNodeInfo(View rootView) { + return AccessibilityNodeInfo.obtain(rootView); + } + @VisibleForTesting public AccessibilityNodeInfo obtainAccessibilityNodeInfo(View rootView, int virtualViewId) { return AccessibilityNodeInfo.obtain(rootView, virtualViewId); @@ -608,6 +615,7 @@ public AccessibilityNodeInfo obtainAccessibilityNodeInfo(View rootView, int virt // Suppressing Lint warning for new API, as we are version guarding all calls to newer APIs @SuppressLint("NewApi") public AccessibilityNodeInfo createAccessibilityNodeInfo(int virtualViewId) { + Log.e(TAG, "virtualViewId " + virtualViewId + " stacktrace"); setAccessibleNavigation(true); if (virtualViewId >= MIN_ENGINE_GENERATED_NODE_ID) { // The node is in the engine generated range, and is provided by the accessibility view @@ -616,13 +624,15 @@ public AccessibilityNodeInfo createAccessibilityNodeInfo(int virtualViewId) { } if (virtualViewId == View.NO_ID) { - AccessibilityNodeInfo result = AccessibilityNodeInfo.obtain(rootAccessibilityView); + AccessibilityNodeInfo result = obtainAccessibilityNodeInfo(rootAccessibilityView); rootAccessibilityView.onInitializeAccessibilityNodeInfo(result); // TODO(mattcarroll): what does it mean for the semantics tree to contain or not contain // the root node ID? if (flutterSemanticsTree.containsKey(ROOT_NODE_ID)) { result.addChild(rootAccessibilityView, ROOT_NODE_ID); } + result.setImportantForAccessibility(false); + // Log.d("myapp", Log.getStackTraceString(new Exception())); return result; } @@ -653,6 +663,11 @@ public AccessibilityNodeInfo createAccessibilityNodeInfo(int virtualViewId) { AccessibilityNodeInfo result = obtainAccessibilityNodeInfo(rootAccessibilityView, virtualViewId); + + // Accessibility Scanner uses isImportantForAccessibility to decide whether to check + // or skip this node. + result.setImportantForAccessibility(isImportant(semanticsNode)); + // Work around for https://github.com/flutter/flutter/issues/2101 if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR2) { result.setViewIdResourceName(""); @@ -983,6 +998,19 @@ && shouldSetCollectionInfo(semanticsNode)) { return result; } + private boolean isImportant(SemanticsNode node) { + if (node.hasFlag(Flag.SCOPES_ROUTE)) { + return false; + } + + if (node.getValueLabelHint() != null) { + return true; + } + + // Return ture if has any user action. + return (node.actions & ~systemAction) != 0; + } + /** * Get the bounds in screen with root FlutterView's offset. * @@ -2141,6 +2169,11 @@ public enum Action { } } + // Actions that are triggered by Android OS, as oppsite to user-triggered actions. + static int systemAction = Action.DID_GAIN_ACCESSIBILITY_FOCUS.value & + Action.DID_LOSE_ACCESSIBILITY_FOCUS.value & + Action.SHOW_ON_SCREEN.value; + // Must match SemanticsFlag in semantics.dart // https://github.com/flutter/engine/blob/main/lib/ui/semantics.dart /* Package */ enum Flag { diff --git a/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java b/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java index 434843e1a8748..d22ac47d5a3b1 100644 --- a/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java +++ b/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java @@ -49,6 +49,7 @@ import io.flutter.embedding.engine.systemchannels.AccessibilityChannel; import io.flutter.plugin.common.BasicMessageChannel; import io.flutter.plugin.platform.PlatformViewsAccessibilityDelegate; +import io.flutter.view.AccessibilityBridge.Action; import io.flutter.view.AccessibilityBridge.Flag; import java.nio.ByteBuffer; import java.nio.charset.Charset; @@ -321,6 +322,130 @@ public void itSetsTraversalAfter() { verify(mockNodeInfo2, times(1)).setTraversalAfter(eq(mockRootView), eq(1)); } + @Test + public void itSetsRootViewNotImportantForAccessibility() { + AccessibilityViewEmbedder mockViewEmbedder = mock(AccessibilityViewEmbedder.class); + AccessibilityManager mockManager = mock(AccessibilityManager.class); + View mockRootView = mock(View.class); + Context context = mock(Context.class); + when(mockRootView.getContext()).thenReturn(context); + when(context.getPackageName()).thenReturn("test"); + AccessibilityBridge accessibilityBridge = + setUpBridge(mockRootView, mockManager, mockViewEmbedder); + ViewParent mockParent = mock(ViewParent.class); + when(mockRootView.getParent()).thenReturn(mockParent); + when(mockManager.isEnabled()).thenReturn(true); + + TestSemanticsNode root = new TestSemanticsNode(); + root.id = 0; + TestSemanticsUpdate testSemanticsUpdate = root.toUpdate(); + testSemanticsUpdate.sendUpdateToBridge(accessibilityBridge); + + AccessibilityBridge spyAccessibilityBridge = spy(accessibilityBridge); + AccessibilityNodeInfo mockNodeInfo = mock(AccessibilityNodeInfo.class); + + when(spyAccessibilityBridge.obtainAccessibilityNodeInfo(mockRootView)) + .thenReturn(mockNodeInfo); + spyAccessibilityBridge.createAccessibilityNodeInfo(View.NO_ID); + verify(mockNodeInfo, times(1)).setImportantForAccessibility(eq(false)); + } + + @Test + public void itSetsNodeImportantForAccessibilityIfItHasContent() { + AccessibilityViewEmbedder mockViewEmbedder = mock(AccessibilityViewEmbedder.class); + AccessibilityManager mockManager = mock(AccessibilityManager.class); + View mockRootView = mock(View.class); + Context context = mock(Context.class); + when(mockRootView.getContext()).thenReturn(context); + when(context.getPackageName()).thenReturn("test"); + AccessibilityBridge accessibilityBridge = + setUpBridge(mockRootView, mockManager, mockViewEmbedder); + ViewParent mockParent = mock(ViewParent.class); + when(mockRootView.getParent()).thenReturn(mockParent); + when(mockManager.isEnabled()).thenReturn(true); + + TestSemanticsNode root = new TestSemanticsNode(); + root.id = 0; + root.label = "some label"; + TestSemanticsUpdate testSemanticsUpdate = root.toUpdate(); + testSemanticsUpdate.sendUpdateToBridge(accessibilityBridge); + + AccessibilityBridge spyAccessibilityBridge = spy(accessibilityBridge); + AccessibilityNodeInfo mockNodeInfo = mock(AccessibilityNodeInfo.class); + + when(spyAccessibilityBridge.obtainAccessibilityNodeInfo(mockRootView, 0)) + .thenReturn(mockNodeInfo); + spyAccessibilityBridge.createAccessibilityNodeInfo(0); + verify(mockNodeInfo, times(1)).setImportantForAccessibility(eq(true)); + } + + @Test + public void itSetsNodeImportantForAccessibilityIfItHasActions() { + AccessibilityViewEmbedder mockViewEmbedder = mock(AccessibilityViewEmbedder.class); + AccessibilityManager mockManager = mock(AccessibilityManager.class); + View mockRootView = mock(View.class); + Context context = mock(Context.class); + when(mockRootView.getContext()).thenReturn(context); + when(context.getPackageName()).thenReturn("test"); + AccessibilityBridge accessibilityBridge = + setUpBridge(mockRootView, mockManager, mockViewEmbedder); + ViewParent mockParent = mock(ViewParent.class); + when(mockRootView.getParent()).thenReturn(mockParent); + when(mockManager.isEnabled()).thenReturn(true); + + TestSemanticsNode root = new TestSemanticsNode(); + root.id = 0; + root.addAction(Action.TAP); + TestSemanticsUpdate testSemanticsUpdate = root.toUpdate(); + testSemanticsUpdate.sendUpdateToBridge(accessibilityBridge); + + AccessibilityBridge spyAccessibilityBridge = spy(accessibilityBridge); + AccessibilityNodeInfo mockNodeInfo = mock(AccessibilityNodeInfo.class); + + when(spyAccessibilityBridge.obtainAccessibilityNodeInfo(mockRootView, 0)) + .thenReturn(mockNodeInfo); + spyAccessibilityBridge.createAccessibilityNodeInfo(0); + verify(mockNodeInfo, times(1)).setImportantForAccessibility(eq(true)); + } + + @Test + public void itSetsNodeUnImportantForAccessibilityIfItIsEmpty() { + AccessibilityViewEmbedder mockViewEmbedder = mock(AccessibilityViewEmbedder.class); + AccessibilityManager mockManager = mock(AccessibilityManager.class); + View mockRootView = mock(View.class); + Context context = mock(Context.class); + when(mockRootView.getContext()).thenReturn(context); + when(context.getPackageName()).thenReturn("test"); + AccessibilityBridge accessibilityBridge = + setUpBridge(mockRootView, mockManager, mockViewEmbedder); + ViewParent mockParent = mock(ViewParent.class); + when(mockRootView.getParent()).thenReturn(mockParent); + when(mockManager.isEnabled()).thenReturn(true); + + TestSemanticsNode root = new TestSemanticsNode(); + root.id = 0; + TestSemanticsNode node = new TestSemanticsNode(); + node.id = 1; + root.children.add(node); + TestSemanticsUpdate testSemanticsUpdate = root.toUpdate(); + testSemanticsUpdate.sendUpdateToBridge(accessibilityBridge); + + AccessibilityBridge spyAccessibilityBridge = spy(accessibilityBridge); + AccessibilityNodeInfo mockNodeInfo = mock(AccessibilityNodeInfo.class); + + when(spyAccessibilityBridge.obtainAccessibilityNodeInfo(mockRootView, 0)) + .thenReturn(mockNodeInfo); + spyAccessibilityBridge.createAccessibilityNodeInfo(0); + verify(mockNodeInfo, times(1)).setImportantForAccessibility(eq(false)); + + AccessibilityNodeInfo mockNodeInfo1 = mock(AccessibilityNodeInfo.class); + + when(spyAccessibilityBridge.obtainAccessibilityNodeInfo(mockRootView, 1)) + .thenReturn(mockNodeInfo1); + spyAccessibilityBridge.createAccessibilityNodeInfo(1); + verify(mockNodeInfo1, times(1)).setImportantForAccessibility(eq(false)); + } + @TargetApi(28) @Test public void itSetCutoutInsetBasedonLayoutModeNever() { From f1c9ea43822454cbde533e7dd58ed4d3a25a6151 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Mon, 7 Aug 2023 11:34:17 -0700 Subject: [PATCH 2/8] update --- .../android/io/flutter/view/AccessibilityBridge.java | 8 ++++---- .../test/io/flutter/view/AccessibilityBridgeTest.java | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/shell/platform/android/io/flutter/view/AccessibilityBridge.java b/shell/platform/android/io/flutter/view/AccessibilityBridge.java index f7e05d59f28ec..75ef0ab80d5b0 100644 --- a/shell/platform/android/io/flutter/view/AccessibilityBridge.java +++ b/shell/platform/android/io/flutter/view/AccessibilityBridge.java @@ -41,7 +41,6 @@ import io.flutter.util.Predicate; import io.flutter.util.ViewUtils; import io.flutter.view.AccessibilityBridge.Flag; - import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.Charset; @@ -2170,9 +2169,10 @@ public enum Action { } // Actions that are triggered by Android OS, as oppsite to user-triggered actions. - static int systemAction = Action.DID_GAIN_ACCESSIBILITY_FOCUS.value & - Action.DID_LOSE_ACCESSIBILITY_FOCUS.value & - Action.SHOW_ON_SCREEN.value; + static int systemAction = + Action.DID_GAIN_ACCESSIBILITY_FOCUS.value + & Action.DID_LOSE_ACCESSIBILITY_FOCUS.value + & Action.SHOW_ON_SCREEN.value; // Must match SemanticsFlag in semantics.dart // https://github.com/flutter/engine/blob/main/lib/ui/semantics.dart diff --git a/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java b/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java index d22ac47d5a3b1..ed1ef208d45a0 100644 --- a/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java +++ b/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java @@ -344,8 +344,7 @@ public void itSetsRootViewNotImportantForAccessibility() { AccessibilityBridge spyAccessibilityBridge = spy(accessibilityBridge); AccessibilityNodeInfo mockNodeInfo = mock(AccessibilityNodeInfo.class); - when(spyAccessibilityBridge.obtainAccessibilityNodeInfo(mockRootView)) - .thenReturn(mockNodeInfo); + when(spyAccessibilityBridge.obtainAccessibilityNodeInfo(mockRootView)).thenReturn(mockNodeInfo); spyAccessibilityBridge.createAccessibilityNodeInfo(View.NO_ID); verify(mockNodeInfo, times(1)).setImportantForAccessibility(eq(false)); } From c10dbdea0454eac67c2af34925fe4b0727f205ba Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Mon, 7 Aug 2023 11:39:16 -0700 Subject: [PATCH 3/8] format --- .../platform/android/io/flutter/view/AccessibilityBridge.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/shell/platform/android/io/flutter/view/AccessibilityBridge.java b/shell/platform/android/io/flutter/view/AccessibilityBridge.java index 75ef0ab80d5b0..757cf7265c75b 100644 --- a/shell/platform/android/io/flutter/view/AccessibilityBridge.java +++ b/shell/platform/android/io/flutter/view/AccessibilityBridge.java @@ -40,7 +40,6 @@ import io.flutter.plugin.platform.PlatformViewsAccessibilityDelegate; import io.flutter.util.Predicate; import io.flutter.util.ViewUtils; -import io.flutter.view.AccessibilityBridge.Flag; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.Charset; @@ -614,7 +613,6 @@ public AccessibilityNodeInfo obtainAccessibilityNodeInfo(View rootView, int virt // Suppressing Lint warning for new API, as we are version guarding all calls to newer APIs @SuppressLint("NewApi") public AccessibilityNodeInfo createAccessibilityNodeInfo(int virtualViewId) { - Log.e(TAG, "virtualViewId " + virtualViewId + " stacktrace"); setAccessibleNavigation(true); if (virtualViewId >= MIN_ENGINE_GENERATED_NODE_ID) { // The node is in the engine generated range, and is provided by the accessibility view @@ -631,7 +629,6 @@ public AccessibilityNodeInfo createAccessibilityNodeInfo(int virtualViewId) { result.addChild(rootAccessibilityView, ROOT_NODE_ID); } result.setImportantForAccessibility(false); - // Log.d("myapp", Log.getStackTraceString(new Exception())); return result; } From 9ee7c31bdead4df5a6a0a2b376b04c4b6f5da096 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Mon, 7 Aug 2023 13:31:54 -0700 Subject: [PATCH 4/8] update --- shell/platform/android/io/flutter/view/AccessibilityBridge.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/view/AccessibilityBridge.java b/shell/platform/android/io/flutter/view/AccessibilityBridge.java index 757cf7265c75b..46c6bb3aec6ac 100644 --- a/shell/platform/android/io/flutter/view/AccessibilityBridge.java +++ b/shell/platform/android/io/flutter/view/AccessibilityBridge.java @@ -2165,7 +2165,7 @@ public enum Action { } } - // Actions that are triggered by Android OS, as oppsite to user-triggered actions. + // Actions that are triggered by Android OS, as opposite to user-triggered actions. static int systemAction = Action.DID_GAIN_ACCESSIBILITY_FOCUS.value & Action.DID_LOSE_ACCESSIBILITY_FOCUS.value From 16a9312584c264c476061607bc532f81a3ecfe88 Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Tue, 8 Aug 2023 08:37:02 -0700 Subject: [PATCH 5/8] Update shell/platform/android/io/flutter/view/AccessibilityBridge.java --- shell/platform/android/io/flutter/view/AccessibilityBridge.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/view/AccessibilityBridge.java b/shell/platform/android/io/flutter/view/AccessibilityBridge.java index 46c6bb3aec6ac..04a09176b54cd 100644 --- a/shell/platform/android/io/flutter/view/AccessibilityBridge.java +++ b/shell/platform/android/io/flutter/view/AccessibilityBridge.java @@ -1003,7 +1003,7 @@ private boolean isImportant(SemanticsNode node) { return true; } - // Return ture if has any user action. + // Return true if has any user action. return (node.actions & ~systemAction) != 0; } From a6ec0868e06a33724d587ecbf3d30e7784432c08 Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Tue, 8 Aug 2023 08:37:27 -0700 Subject: [PATCH 6/8] Update shell/platform/android/io/flutter/view/AccessibilityBridge.java --- shell/platform/android/io/flutter/view/AccessibilityBridge.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/view/AccessibilityBridge.java b/shell/platform/android/io/flutter/view/AccessibilityBridge.java index 04a09176b54cd..21007d3b5cb7d 100644 --- a/shell/platform/android/io/flutter/view/AccessibilityBridge.java +++ b/shell/platform/android/io/flutter/view/AccessibilityBridge.java @@ -1003,7 +1003,7 @@ private boolean isImportant(SemanticsNode node) { return true; } - // Return true if has any user action. + // Return true if it has any user action. return (node.actions & ~systemAction) != 0; } From 5352976d5141d7ab5f58e384a33b938043eb844a Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Tue, 8 Aug 2023 14:27:02 -0700 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Reid Baker --- .../android/io/flutter/view/AccessibilityBridge.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/view/AccessibilityBridge.java b/shell/platform/android/io/flutter/view/AccessibilityBridge.java index 21007d3b5cb7d..4bb87231e5263 100644 --- a/shell/platform/android/io/flutter/view/AccessibilityBridge.java +++ b/shell/platform/android/io/flutter/view/AccessibilityBridge.java @@ -1003,7 +1003,7 @@ private boolean isImportant(SemanticsNode node) { return true; } - // Return true if it has any user action. + // Return true if the node has had any user action (not including system actions) return (node.actions & ~systemAction) != 0; } @@ -2165,7 +2165,9 @@ public enum Action { } } - // Actions that are triggered by Android OS, as opposite to user-triggered actions. + // Actions that are triggered by Android OS, as opposed to user-triggered actions. + // + // This int is intended to be use in a bitwise comparison. static int systemAction = Action.DID_GAIN_ACCESSIBILITY_FOCUS.value & Action.DID_LOSE_ACCESSIBILITY_FOCUS.value From 50d6b02c055e257ba5245e7afd83266e9001e8fe Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Wed, 9 Aug 2023 09:18:41 -0700 Subject: [PATCH 8/8] format --- .../platform/android/io/flutter/view/AccessibilityBridge.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/view/AccessibilityBridge.java b/shell/platform/android/io/flutter/view/AccessibilityBridge.java index 4bb87231e5263..565cfcad6ca13 100644 --- a/shell/platform/android/io/flutter/view/AccessibilityBridge.java +++ b/shell/platform/android/io/flutter/view/AccessibilityBridge.java @@ -1003,7 +1003,7 @@ private boolean isImportant(SemanticsNode node) { return true; } - // Return true if the node has had any user action (not including system actions) + // Return true if the node has had any user action (not including system actions) return (node.actions & ~systemAction) != 0; } @@ -2167,7 +2167,7 @@ public enum Action { // Actions that are triggered by Android OS, as opposed to user-triggered actions. // - // This int is intended to be use in a bitwise comparison. + // This int is intended to be use in a bitwise comparison. static int systemAction = Action.DID_GAIN_ACCESSIBILITY_FOCUS.value & Action.DID_LOSE_ACCESSIBILITY_FOCUS.value