Fix accessibility focus loss when first focusing on text field (flutt…

…er#17803)
christopherfujino · Apr 24, 2020 · 2589d07 · 2589d07
1 parent 3af2b1a
commit 2589d07
Show file tree

Hide file tree

Showing 19 changed files with 427 additions and 102 deletions.
diff --git a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm
@@ -780,22 +780,16 @@ - (void)deleteBackward {
     [self replaceRange:_selectedTextRange withText:@""];
 }
 
-@end
-
-/**
- * Hides `FlutterTextInputView` from iOS accessibility system so it
- * does not show up twice, once where it is in the `UIView` hierarchy,
- * and a second time as part of the `SemanticsObject` hierarchy.
- */
-@interface FlutterTextInputViewAccessibilityHider : UIView {
-}
-
-@end
-
-@implementation FlutterTextInputViewAccessibilityHider {
-}
-
 - (BOOL)accessibilityElementsHidden {
+  // We are hiding this accessibility element.
+  // There are 2 accessible elements involved in text entry in 2 different parts of the view
+  // hierarchy. This `FlutterTextInputView` is injected at the top of key window. We use this as a
+  // `UITextInput` protocol to bridge text edit events between Flutter and iOS.
+  //
+  // We also create ur own custom `UIAccessibilityElements` tree with our `SemanticsObject` to
+  // mimic the semantics tree from Flutter. We want the text field to be represented as a
+  // `TextInputSemanticsObject` in that `SemanticsObject` tree rather than in this
+  // `FlutterTextInputView` bridge which doesn't appear above a text field from the Flutter side.
   return YES;
 }
 
@@ -806,7 +800,6 @@ @interface FlutterTextInputPlugin ()
 @property(nonatomic, retain) FlutterTextInputView* nonAutofillSecureInputView;
 @property(nonatomic, retain) NSMutableArray<FlutterTextInputView*>* inputViews;
 @property(nonatomic, assign) FlutterTextInputView* activeView;
-@property(nonatomic, retain) FlutterTextInputViewAccessibilityHider* inputHider;
 @end
 
 @implementation FlutterTextInputPlugin
@@ -824,7 +817,6 @@ - (instancetype)init {
     _inputViews = [[NSMutableArray alloc] init];
 
     _activeView = _nonAutofillInputView;
-    _inputHider = [[FlutterTextInputViewAccessibilityHider alloc] init];
   }
 
   return self;
@@ -834,7 +826,6 @@ - (void)dealloc {
   [self hideTextInput];
   [_nonAutofillInputView release];
   [_nonAutofillSecureInputView release];
-  [_inputHider release];
   [_inputViews release];
 
   [super dealloc];
@@ -873,19 +864,19 @@ - (void)showTextInput {
            @"The application must have a key window since the keyboard client "
            @"must be part of the responder chain to function");
   _activeView.textInputDelegate = _textInputDelegate;
-  if (![_activeView isDescendantOfView:_inputHider]) {
-    [_inputHider addSubview:_activeView];
+
+  if (_activeView.window != keyWindow) {
+    [keyWindow addSubview:_activeView];
   }
-  [keyWindow addSubview:_inputHider];
   [_activeView becomeFirstResponder];
 }
 
 - (void)hideTextInput {
   [_activeView resignFirstResponder];
-  [_inputHider removeFromSuperview];
 }
 
 - (void)setTextInputClient:(int)client withConfiguration:(NSDictionary*)configuration {
+  UIWindow* keyWindow = [UIApplication sharedApplication].keyWindow;
   NSArray* fields = configuration[@"fields"];
   NSString* clientUniqueId = uniqueIdFromDictionary(configuration);
   bool isSecureTextEntry = [configuration[@"obscureText"] boolValue];
@@ -894,16 +885,19 @@ - (void)setTextInputClient:(int)client withConfiguration:(NSDictionary*)configur
     _activeView = isSecureTextEntry ? _nonAutofillSecureInputView : _nonAutofillInputView;
     [FlutterTextInputPlugin setupInputView:_activeView withConfiguration:configuration];
 
-    if (![_activeView isDescendantOfView:_inputHider]) {
-      [_inputHider addSubview:_activeView];
+    if (_activeView.window != keyWindow) {
+      [keyWindow addSubview:_activeView];
     }
   } else {
     NSAssert(clientUniqueId != nil, @"The client's unique id can't be null");
     for (FlutterTextInputView* view in _inputViews) {
       [view removeFromSuperview];
     }
-    for (UIView* subview in _inputHider.subviews) {
-      [subview removeFromSuperview];
+
+    for (UIView* view in keyWindow.subviews) {
+      if ([view isKindOfClass:[FlutterTextInputView class]]) {
+        [view removeFromSuperview];
+      }
     }
 
     [_inputViews removeAllObjects];
@@ -921,7 +915,7 @@ - (void)setTextInputClient:(int)client withConfiguration:(NSDictionary*)configur
       }
 
       [FlutterTextInputPlugin setupInputView:newInputView withConfiguration:field];
-      [_inputHider addSubview:newInputView];
+      [keyWindow addSubview:newInputView];
     }
   }
 

diff --git a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m
@@ -38,18 +38,30 @@ - (void)testSecureInput {
                              result:^(id _Nullable result){
                              }];
 
-  // Find all input views in the input hider view.
-  NSArray<FlutterTextInputView*>* inputFields =
-      [[[textInputPlugin textInputView] superview] subviews];
-
-  // Find the inactive autofillable input field.
+  // Find all the FlutterTextInputViews we created.
+  NSArray<FlutterTextInputView*>* inputFields = [[[[textInputPlugin textInputView] superview]
+      subviews]
+      filteredArrayUsingPredicate:[NSPredicate predicateWithFormat:@"class == %@",
+                                                                   [FlutterTextInputView class]]];
+
+  // There are no autofill and the mock framework requested a secure entry. The first and only
+  // inserted FlutterTextInputView should be a secure text entry one.
   FlutterTextInputView* inputView = inputFields[0];
 
   // Verify secureTextEntry is set to the correct value.
   XCTAssertTrue(inputView.secureTextEntry);
 
-  // Clean up mocks
+  // We should have only ever created one FlutterTextInputView.
+  XCTAssertEqual(inputFields.count, 1);
+
+  // The one FlutterTextInputView we inserted into the view hierarchy should be the text input
+  // plugin's active text input view.
+  XCTAssertEqual(inputView, textInputPlugin.textInputView);
+
+  // Clean up.
   [engine stopMocking];
+  [[[[textInputPlugin textInputView] superview] subviews]
+      makeObjectsPerformSelector:@selector(removeFromSuperview)];
 }
 - (void)testAutofillInputViews {
   // Setup test.
@@ -94,9 +106,11 @@ - (void)testAutofillInputViews {
                              result:^(id _Nullable result){
                              }];
 
-  // Find all input views in the input hider view.
-  NSArray<FlutterTextInputView*>* inputFields =
-      [[[textInputPlugin textInputView] superview] subviews];
+  // Find all the FlutterTextInputViews we created.
+  NSArray<FlutterTextInputView*>* inputFields = [[[[textInputPlugin textInputView] superview]
+      subviews]
+      filteredArrayUsingPredicate:[NSPredicate predicateWithFormat:@"class == %@",
+                                                                   [FlutterTextInputView class]]];
 
   XCTAssertEqual(inputFields.count, 2);
 
@@ -108,8 +122,10 @@ - (void)testAutofillInputViews {
   // Verify behavior.
   OCMVerify([engine updateEditingClient:0 withState:[OCMArg isNotNil] withTag:@"field2"]);
 
-  // Clean up mocks
+  // Clean up.
   [engine stopMocking];
+  [[[[textInputPlugin textInputView] superview] subviews]
+      makeObjectsPerformSelector:@selector(removeFromSuperview)];
 }
 
 - (void)testAutocorrectionPromptRectAppears {

diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm
@@ -4,7 +4,7 @@
 
 #import <OCMock/OCMock.h>
 #import <XCTest/XCTest.h>
-#include "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"
+#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h"
 #import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h"
 #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h"
 

diff --git a/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/xcshareddata/xcschemes/IosUnitTests.xcscheme b/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/xcshareddata/xcschemes/IosUnitTests.xcscheme
@@ -0,0 +1,88 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Scheme
+   LastUpgradeVersion = "1140"
+   version = "1.3">
+   <BuildAction
+      parallelizeBuildables = "YES"
+      buildImplicitDependencies = "YES">
+      <BuildActionEntries>
+         <BuildActionEntry
+            buildForTesting = "YES"
+            buildForRunning = "YES"
+            buildForProfiling = "YES"
+            buildForArchiving = "YES"
+            buildForAnalyzing = "YES">
+            <BuildableReference
+               BuildableIdentifier = "primary"
+               BlueprintIdentifier = "0D6AB6B022BB05E100EEE540"
+               BuildableName = "IosUnitTests.app"
+               BlueprintName = "IosUnitTests"
+               ReferencedContainer = "container:IosUnitTests.xcodeproj">
+            </BuildableReference>
+         </BuildActionEntry>
+      </BuildActionEntries>
+   </BuildAction>
+   <TestAction
+      buildConfiguration = "Debug"
+      selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
+      selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
+      shouldUseLaunchSchemeArgsEnv = "YES">
+      <Testables>
+         <TestableReference
+            skipped = "NO">
+            <BuildableReference
+               BuildableIdentifier = "primary"
+               BlueprintIdentifier = "0D6AB6C822BB05E200EEE540"
+               BuildableName = "IosUnitTestsTests.xctest"
+               BlueprintName = "IosUnitTestsTests"
+               ReferencedContainer = "container:IosUnitTests.xcodeproj">
+            </BuildableReference>
+         </TestableReference>
+      </Testables>
+   </TestAction>
+   <LaunchAction
+      buildConfiguration = "Debug"
+      selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
+      selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
+      launchStyle = "0"
+      useCustomWorkingDirectory = "NO"
+      ignoresPersistentStateOnLaunch = "NO"
+      debugDocumentVersioning = "YES"
+      debugServiceExtension = "internal"
+      allowLocationSimulation = "YES">
+      <BuildableProductRunnable
+         runnableDebuggingMode = "0">
+         <BuildableReference
+            BuildableIdentifier = "primary"
+            BlueprintIdentifier = "0D6AB6B022BB05E100EEE540"
+            BuildableName = "IosUnitTests.app"
+            BlueprintName = "IosUnitTests"
+            ReferencedContainer = "container:IosUnitTests.xcodeproj">
+         </BuildableReference>
+      </BuildableProductRunnable>
+   </LaunchAction>
+   <ProfileAction
+      buildConfiguration = "Release"
+      shouldUseLaunchSchemeArgsEnv = "YES"
+      savedToolIdentifier = ""
+      useCustomWorkingDirectory = "NO"
+      debugDocumentVersioning = "YES">
+      <BuildableProductRunnable
+         runnableDebuggingMode = "0">
+         <BuildableReference
+            BuildableIdentifier = "primary"
+            BlueprintIdentifier = "0D6AB6B022BB05E100EEE540"
+            BuildableName = "IosUnitTests.app"
+            BlueprintName = "IosUnitTests"
+            ReferencedContainer = "container:IosUnitTests.xcodeproj">
+         </BuildableReference>
+      </BuildableProductRunnable>
+   </ProfileAction>
+   <AnalyzeAction
+      buildConfiguration = "Debug">
+   </AnalyzeAction>
+   <ArchiveAction
+      buildConfiguration = "Release"
+      revealArchiveInOrganizer = "YES">
+   </ArchiveAction>
+</Scheme>
diff --git a/...nario_app/android/app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java b/...nario_app/android/app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java
@@ -34,15 +34,16 @@ public void smokeTestEngineLaunch() throws Throwable {
     UiThreadStatement.runOnUiThread(() -> engine.set(new FlutterEngine(applicationContext)));
     CompletableFuture<Boolean> statusReceived = new CompletableFuture<>();
 
-    // The default Dart main entrypoint sends back a platform message on the "scenario_status"
+    // The default Dart main entrypoint sends back a platform message on the "waiting_for_status"
     // channel. That will be our launch success assertion condition.
     engine
         .get()
         .getDartExecutor()
         .setMessageHandler(
-            "scenario_status", (byteBuffer, binaryReply) -> statusReceived.complete(Boolean.TRUE));
+            "waiting_for_status",
+            (byteBuffer, binaryReply) -> statusReceived.complete(Boolean.TRUE));
 
-    // Launching the entrypoint will run the Dart code that sends the "scenario_status" platform
+    // Launching the entrypoint will run the Dart code that sends the "waiting_for_status" platform
     // message.
     UiThreadStatement.runOnUiThread(
         () ->
@@ -54,7 +55,7 @@ public void smokeTestEngineLaunch() throws Throwable {
     try {
       Boolean result = statusReceived.get(10, TimeUnit.SECONDS);
       if (!result) {
-        fail("expected message on scenario_status not received");
+        fail("expected message on waiting_for_status not received");
       }
     } catch (ExecutionException e) {
       fail(e.getMessage());

diff --git a/testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj b/testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj
@@ -7,6 +7,7 @@
 	objects = {
 
 /* Begin PBXBuildFile section */
+		0A42BFB42447E179007E212E /* TextSemanticsFocusTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 0A42BFB32447E179007E212E /* TextSemanticsFocusTest.m */; };
 		0A57B3BD2323C4BD00DD9521 /* ScreenBeforeFlutter.m in Sources */ = {isa = PBXBuildFile; fileRef = 0A57B3BC2323C4BD00DD9521 /* ScreenBeforeFlutter.m */; };
 		0A57B3BF2323C74200DD9521 /* FlutterEngine+ScenariosTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 0A57B3BE2323C74200DD9521 /* FlutterEngine+ScenariosTest.m */; };
 		0A57B3C22323D2D700DD9521 /* AppLifecycleTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 0A57B3C12323D2D700DD9521 /* AppLifecycleTests.m */; };
@@ -110,6 +111,8 @@
 /* End PBXCopyFilesBuildPhase section */
 
 /* Begin PBXFileReference section */
+		0A42BFB32447E179007E212E /* TextSemanticsFocusTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = TextSemanticsFocusTest.m; sourceTree = "<group>"; };
+		0A42BFB52447E19F007E212E /* TextSemanticsFocusTest.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = TextSemanticsFocusTest.h; sourceTree = "<group>"; };
 		0A57B3BB2323C4BD00DD9521 /* ScreenBeforeFlutter.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ScreenBeforeFlutter.h; sourceTree = "<group>"; };
 		0A57B3BC2323C4BD00DD9521 /* ScreenBeforeFlutter.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ScreenBeforeFlutter.m; sourceTree = "<group>"; };
 		0A57B3BE2323C74200DD9521 /* FlutterEngine+ScenariosTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "FlutterEngine+ScenariosTest.m"; sourceTree = "<group>"; };
@@ -280,6 +283,8 @@
 				68A5B63323EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m */,
 				0D8470A2240F0B1F0030B565 /* StatusBarTest.h */,
 				0D8470A3240F0B1F0030B565 /* StatusBarTest.m */,
+				0A42BFB32447E179007E212E /* TextSemanticsFocusTest.m */,
+				0A42BFB52447E19F007E212E /* TextSemanticsFocusTest.h */,
 			);
 			path = ScenariosUITests;
 			sourceTree = "<group>";
@@ -498,6 +503,7 @@
 				6816DBA42318358200A51400 /* PlatformViewGoldenTestManager.m in Sources */,
 				248D76EF22E388380012F0C1 /* PlatformViewUITests.m in Sources */,
 				0D8470A4240F0B1F0030B565 /* StatusBarTest.m in Sources */,
+				0A42BFB42447E179007E212E /* TextSemanticsFocusTest.m in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};

diff --git a/.../scenario_app/ios/Scenarios/Scenarios.xcodeproj/xcshareddata/xcschemes/Scenarios.xcscheme b/.../scenario_app/ios/Scenarios/Scenarios.xcodeproj/xcshareddata/xcschemes/Scenarios.xcscheme
@@ -27,6 +27,15 @@
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
       shouldUseLaunchSchemeArgsEnv = "YES">
+      <MacroExpansion>
+         <BuildableReference
+            BuildableIdentifier = "primary"
+            BlueprintIdentifier = "248D76C622E388370012F0C1"
+            BuildableName = "Scenarios.app"
+            BlueprintName = "Scenarios"
+            ReferencedContainer = "container:Scenarios.xcodeproj">
+         </BuildableReference>
+      </MacroExpansion>
       <Testables>
          <TestableReference
             skipped = "NO"
@@ -51,17 +60,6 @@
             </BuildableReference>
          </TestableReference>
       </Testables>
-      <MacroExpansion>
-         <BuildableReference
-            BuildableIdentifier = "primary"
-            BlueprintIdentifier = "248D76C622E388370012F0C1"
-            BuildableName = "Scenarios.app"
-            BlueprintName = "Scenarios"
-            ReferencedContainer = "container:Scenarios.xcodeproj">
-         </BuildableReference>
-      </MacroExpansion>
-      <AdditionalOptions>
-      </AdditionalOptions>
    </TestAction>
    <LaunchAction
       buildConfiguration = "Debug"
@@ -88,13 +86,15 @@
             argument = "--screen-before-flutter"
             isEnabled = "NO">
          </CommandLineArgument>
+         <CommandLineArgument
+            argument = "--text-semantics-focus"
+            isEnabled = "NO">
+         </CommandLineArgument>
          <CommandLineArgument
             argument = "--platform-view"
             isEnabled = "NO">
          </CommandLineArgument>
       </CommandLineArguments>
-      <AdditionalOptions>
-      </AdditionalOptions>
    </LaunchAction>
    <ProfileAction
       buildConfiguration = "Release"