Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear focus if a platform view goes away #17381

Merged
merged 7 commits into from
Apr 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions shell/platform/android/io/flutter/view/AccessibilityBridge.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.RequiresApi;
import androidx.annotation.VisibleForTesting;
import io.flutter.BuildConfig;
import io.flutter.embedding.engine.systemchannels.AccessibilityChannel;
import io.flutter.plugin.platform.PlatformViewsAccessibilityDelegate;
Expand Down Expand Up @@ -333,10 +334,32 @@ public AccessibilityBridge(
// TODO(mattcarrol): Add the annotation once the plumbing is done.
// https://github.com/flutter/flutter/issues/29618
PlatformViewsAccessibilityDelegate platformViewsAccessibilityDelegate) {
this(
rootAccessibilityView,
accessibilityChannel,
accessibilityManager,
contentResolver,
new AccessibilityViewEmbedder(rootAccessibilityView, MIN_ENGINE_GENERATED_NODE_ID),
platformViewsAccessibilityDelegate);
}

@VisibleForTesting
public AccessibilityBridge(
@NonNull View rootAccessibilityView,
@NonNull AccessibilityChannel accessibilityChannel,
@NonNull AccessibilityManager accessibilityManager,
@NonNull ContentResolver contentResolver,
@NonNull AccessibilityViewEmbedder accessibilityViewEmbedder,
// This should be @NonNull once the plumbing for
// io.flutter.embedding.engine.android.FlutterView is done.
// TODO(mattcarrol): Add the annotation once the plumbing is done.
// https://github.com/flutter/flutter/issues/29618
PlatformViewsAccessibilityDelegate platformViewsAccessibilityDelegate) {
this.rootAccessibilityView = rootAccessibilityView;
this.accessibilityChannel = accessibilityChannel;
this.accessibilityManager = accessibilityManager;
this.contentResolver = contentResolver;
this.accessibilityViewEmbedder = accessibilityViewEmbedder;
this.platformViewsAccessibilityDelegate = platformViewsAccessibilityDelegate;

// Tell Flutter whether accessibility is initially active or not. Then register a listener
Expand Down Expand Up @@ -388,8 +411,6 @@ public void onTouchExplorationStateChanged(boolean isTouchExplorationEnabled) {
if (platformViewsAccessibilityDelegate != null) {
platformViewsAccessibilityDelegate.attachAccessibilityBridge(this);
}
accessibilityViewEmbedder =
new AccessibilityViewEmbedder(rootAccessibilityView, MIN_ENGINE_GENERATED_NODE_ID);
}

/**
Expand Down Expand Up @@ -1580,15 +1601,31 @@ private void willRemoveSemanticsNode(SemanticsNode semanticsNodeToBeRemoved) {
// for null'ing accessibilityFocusedSemanticsNode, inputFocusedSemanticsNode,
// and hoveredObject. Is this a hook method or a command?
semanticsNodeToBeRemoved.parent = null;

if (semanticsNodeToBeRemoved.platformViewId != -1
&& embeddedAccessibilityFocusedNodeId != null
&& accessibilityViewEmbedder.platformViewOfNode(embeddedAccessibilityFocusedNodeId)
== platformViewsAccessibilityDelegate.getPlatformViewById(
semanticsNodeToBeRemoved.platformViewId)) {
// If the currently focused a11y node is within a platform view that is
// getting removed: clear it's a11y focus.
sendAccessibilityEvent(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little concerned about sending an event on behalf of the webview (e.g my concern is that it's internal state may think that it is still focused), do you know whether this is considered safe to do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I don't have an authoritative answer to that question :(

In my experiments it all seems to work out. E.g. when you go back to the route with the webview by popping the route above it, focus behaves reasonable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll live with that if it's the best we got 😕

The other approach I'd consider would be to see if we can make the webview send that event (e.g maybe by turning it invisible within the vd?)

embeddedAccessibilityFocusedNodeId,
AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUS_CLEARED);
embeddedAccessibilityFocusedNodeId = null;
}

if (accessibilityFocusedSemanticsNode == semanticsNodeToBeRemoved) {
sendAccessibilityEvent(
accessibilityFocusedSemanticsNode.id,
AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUS_CLEARED);
accessibilityFocusedSemanticsNode = null;
}

if (inputFocusedSemanticsNode == semanticsNodeToBeRemoved) {
inputFocusedSemanticsNode = null;
}

if (hoveredObject == semanticsNodeToBeRemoved) {
hoveredObject = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
* corresponding platform view and `originId`.
*/
@Keep
final class AccessibilityViewEmbedder {
class AccessibilityViewEmbedder {
private static final String TAG = "AccessibilityBridge";

private final ReflectionAccessors reflectionAccessors;
Expand Down Expand Up @@ -387,6 +387,18 @@ public boolean onAccessibilityHoverEvent(int rootFlutterId, @NonNull MotionEvent
return origin.view.dispatchGenericMotionEvent(translatedEvent);
}

/**
* Returns the View that contains the accessibility node identified by the provided flutterId or
* null if it doesn't belong to a view.
*/
public View platformViewOfNode(int flutterId) {
ViewAndId viewAndId = flutterIdToOrigin.get(flutterId);
if (viewAndId == null) {
return null;
}
return viewAndId.view;
}

private static class ViewAndId {
final View view;
final int id;
Expand Down
140 changes: 121 additions & 19 deletions shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,27 @@
package io.flutter.view;

import static org.junit.Assert.assertEquals;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import android.content.ContentResolver;
import android.content.Context;
import android.view.View;
import android.view.ViewParent;
import android.view.accessibility.AccessibilityEvent;
import android.view.accessibility.AccessibilityManager;
import android.view.accessibility.AccessibilityNodeInfo;
import io.flutter.embedding.engine.systemchannels.AccessibilityChannel;
import io.flutter.plugin.platform.PlatformViewsAccessibilityDelegate;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.annotation.Config;

Expand Down Expand Up @@ -73,24 +80,103 @@ public void itDoesNotContainADescriptionIfScopesRoute() {
assertEquals(nodeInfo.getText(), null);
}

AccessibilityBridge setUpBridge() {
View view = mock(View.class);
@Test
public void itUnfocusesPlatformViewWhenPlatformViewGoesAway() {
AccessibilityViewEmbedder mockViewEmbedder = mock(AccessibilityViewEmbedder.class);
AccessibilityManager mockManager = mock(AccessibilityManager.class);
View mockRootView = mock(View.class);
Context context = mock(Context.class);
when(view.getContext()).thenReturn(context);
when(mockRootView.getContext()).thenReturn(context);
when(context.getPackageName()).thenReturn("test");
AccessibilityChannel accessibilityChannel = mock(AccessibilityChannel.class);
AccessibilityManager accessibilityManager = mock(AccessibilityManager.class);
ContentResolver contentResolver = mock(ContentResolver.class);
PlatformViewsAccessibilityDelegate platformViewsAccessibilityDelegate =
mock(PlatformViewsAccessibilityDelegate.class);
AccessibilityBridge accessibilityBridge =
new AccessibilityBridge(
view,
accessibilityChannel,
accessibilityManager,
contentResolver,
platformViewsAccessibilityDelegate);
return accessibilityBridge;
setUpBridge(mockRootView, mockManager, mockViewEmbedder);

// Sent a11y tree with platform view.
TestSemanticsNode root = new TestSemanticsNode();
root.id = 0;
TestSemanticsNode platformView = new TestSemanticsNode();
platformView.id = 1;
platformView.platformViewId = 42;
root.children.add(platformView);
TestSemanticsUpdate testSemanticsUpdate = root.toUpdate();
accessibilityBridge.updateSemantics(testSemanticsUpdate.buffer, testSemanticsUpdate.strings);

// Set a11y focus to platform view.
View mockView = mock(View.class);
AccessibilityEvent focusEvent = mock(AccessibilityEvent.class);
when(mockViewEmbedder.requestSendAccessibilityEvent(mockView, mockView, focusEvent))
.thenReturn(true);
when(mockViewEmbedder.getRecordFlutterId(mockView, focusEvent)).thenReturn(42);
when(focusEvent.getEventType()).thenReturn(AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUSED);
accessibilityBridge.externalViewRequestSendAccessibilityEvent(mockView, mockView, focusEvent);

// Replace the platform view.
TestSemanticsNode node = new TestSemanticsNode();
node.id = 2;
root.children.clear();
root.children.add(node);
testSemanticsUpdate = root.toUpdate();
when(mockManager.isEnabled()).thenReturn(true);
ViewParent mockParent = mock(ViewParent.class);
when(mockRootView.getParent()).thenReturn(mockParent);
accessibilityBridge.updateSemantics(testSemanticsUpdate.buffer, testSemanticsUpdate.strings);

// Check that unfocus event was sent.
ArgumentCaptor<AccessibilityEvent> eventCaptor =
ArgumentCaptor.forClass(AccessibilityEvent.class);
verify(mockParent, times(2))
.requestSendAccessibilityEvent(eq(mockRootView), eventCaptor.capture());
AccessibilityEvent event = eventCaptor.getAllValues().get(0);
assertEquals(event.getEventType(), AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUS_CLEARED);
}

AccessibilityBridge setUpBridge() {
return setUpBridge(null, null, null, null, null, null);
}

AccessibilityBridge setUpBridge(
View rootAccessibilityView,
AccessibilityManager accessibilityManager,
AccessibilityViewEmbedder accessibilityViewEmbedder) {
return setUpBridge(
rootAccessibilityView, null, accessibilityManager, null, accessibilityViewEmbedder, null);
}

AccessibilityBridge setUpBridge(
View rootAccessibilityView,
AccessibilityChannel accessibilityChannel,
AccessibilityManager accessibilityManager,
ContentResolver contentResolver,
AccessibilityViewEmbedder accessibilityViewEmbedder,
PlatformViewsAccessibilityDelegate platformViewsAccessibilityDelegate) {
if (rootAccessibilityView == null) {
rootAccessibilityView = mock(View.class);
Context context = mock(Context.class);
when(rootAccessibilityView.getContext()).thenReturn(context);
when(context.getPackageName()).thenReturn("test");
}
if (accessibilityChannel == null) {
accessibilityChannel = mock(AccessibilityChannel.class);
}
if (accessibilityManager == null) {
accessibilityManager = mock(AccessibilityManager.class);
}
if (contentResolver == null) {
contentResolver = mock(ContentResolver.class);
}
if (accessibilityViewEmbedder == null) {
accessibilityViewEmbedder = mock(AccessibilityViewEmbedder.class);
}
if (platformViewsAccessibilityDelegate == null) {
platformViewsAccessibilityDelegate = mock(PlatformViewsAccessibilityDelegate.class);
}
return new AccessibilityBridge(
rootAccessibilityView,
accessibilityChannel,
accessibilityManager,
contentResolver,
accessibilityViewEmbedder,
platformViewsAccessibilityDelegate);
}

/// The encoding for semantics is described in platform_view_android.cc
Expand Down Expand Up @@ -136,11 +222,18 @@ void addFlag(AccessibilityBridge.Flag flag) {
float top = 0.0f;
float right = 0.0f;
float bottom = 0.0f;
// children and custom actions not supported.
final List<TestSemanticsNode> children = new ArrayList<TestSemanticsNode>();
// custom actions not supported.

TestSemanticsUpdate toUpdate() {
ArrayList<String> strings = new ArrayList<String>();
ByteBuffer bytes = ByteBuffer.allocate(1000);
addToBuffer(bytes, strings);
bytes.flip();
return new TestSemanticsUpdate(bytes, strings.toArray(new String[strings.size()]));
}

protected void addToBuffer(ByteBuffer bytes, ArrayList<String> strings) {
bytes.putInt(id);
bytes.putInt(flags);
bytes.putInt(actions);
Expand Down Expand Up @@ -169,11 +262,20 @@ TestSemanticsUpdate toUpdate() {
bytes.putFloat(0);
}
// children in traversal order.
bytes.putInt(0);
bytes.putInt(children.size());
for (TestSemanticsNode node : children) {
bytes.putInt(node.id);
}
// children in hit test order.
for (TestSemanticsNode node : children) {
bytes.putInt(node.id);
}
// custom actions
bytes.putInt(0);
bytes.flip();
return new TestSemanticsUpdate(bytes, strings.toArray(new String[strings.size()]));
// child nodes
for (TestSemanticsNode node : children) {
node.addToBuffer(bytes, strings);
}
}
}

Expand Down