Fixed ios layout change to not refocus semantics object if the focus … (flutter#21029)

chunhtai · web-flow · commit 2a12d78c1733 · 2020-09-09T15:39:21.000-07:00
* Fixed ios layout change to not refocus semantics object if the focus is outside of flutter

* update

* addressing comments
diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm
@@ -453,7 +453,7 @@ - (BOOL)accessibilityPerformEscape {
 - (void)accessibilityElementDidBecomeFocused {
   if (![self isAccessibilityBridgeAlive])
     return;
-  [self bridge]->AccessibilityFocusDidChange([self uid]);
+  [self bridge]->AccessibilityObjectDidBecomeFocused([self uid]);
   if ([self node].HasFlag(flutter::SemanticsFlags::kIsHidden) ||
       [self node].HasFlag(flutter::SemanticsFlags::kIsHeader)) {
     [self bridge]->DispatchSemanticsAction([self uid], flutter::SemanticsAction::kShowOnScreen);
@@ -467,6 +467,7 @@ - (void)accessibilityElementDidBecomeFocused {
 - (void)accessibilityElementDidLoseFocus {
   if (![self isAccessibilityBridgeAlive])
     return;
+  [self bridge]->AccessibilityObjectDidLoseFocus([self uid]);
   if ([self node].HasAction(flutter::SemanticsAction::kDidLoseAccessibilityFocus)) {
     [self bridge]->DispatchSemanticsAction([self uid],
                                            flutter::SemanticsAction::kDidLoseAccessibilityFocus);
diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm
@@ -33,7 +33,8 @@ void DispatchSemanticsAction(int32_t id,
     SemanticsActionObservation observation(id, action);
     observations.push_back(observation);
   }
-  void AccessibilityFocusDidChange(int32_t id) override {}
+  void AccessibilityObjectDidBecomeFocused(int32_t id) override {}
+  void AccessibilityObjectDidLoseFocus(int32_t id) override {}
   FlutterPlatformViewsController* GetPlatformViewsController() const override { return nil; }
   std::vector<SemanticsActionObservation> observations;
 
diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.h
@@ -61,7 +61,8 @@ class AccessibilityBridge final : public AccessibilityBridgeIos {
   void DispatchSemanticsAction(int32_t id,
                                flutter::SemanticsAction action,
                                std::vector<uint8_t> args) override;
-  void AccessibilityFocusDidChange(int32_t id) override;
+  void AccessibilityObjectDidBecomeFocused(int32_t id) override;
+  void AccessibilityObjectDidLoseFocus(int32_t id) override;
 
   UIView<UITextInput>* textInputView() override;
 
@@ -85,6 +86,9 @@ class AccessibilityBridge final : public AccessibilityBridgeIos {
   FlutterViewController* view_controller_;
   PlatformViewIOS* platform_view_;
   FlutterPlatformViewsController* platform_views_controller_;
+  // If the this id is kSemanticObjectIdInvalid, it means either nothing has
+  // been focused or the focus is currently outside of the flutter application
+  // (i.e. the status bar or keyboard)
   int32_t last_focused_semantics_object_id_;
   fml::scoped_nsobject<NSMutableDictionary<NSNumber*, SemanticsObject*>> objects_;
   fml::scoped_nsprotocol<FlutterBasicMessageChannel*> accessibility_channel_;
diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm
@@ -16,6 +16,8 @@
 namespace flutter {
 namespace {
 
+constexpr int32_t kSemanticObjectIdInvalid = -1;
+
 class DefaultIosDelegate : public AccessibilityBridge::IosDelegate {
  public:
   bool IsFlutterViewControllerPresentingModalViewController(
@@ -41,7 +43,7 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification,
     : view_controller_(view_controller),
       platform_view_(platform_view),
       platform_views_controller_(platform_views_controller),
-      last_focused_semantics_object_id_(0),
+      last_focused_semantics_object_id_(kSemanticObjectIdInvalid),
       objects_([[NSMutableDictionary alloc] init]),
       weak_factory_(this),
       previous_route_id_(0),
@@ -67,10 +69,16 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification,
   return [[platform_view_->GetOwnerViewController().get().engine textInputPlugin] textInputView];
 }
 
-void AccessibilityBridge::AccessibilityFocusDidChange(int32_t id) {
+void AccessibilityBridge::AccessibilityObjectDidBecomeFocused(int32_t id) {
   last_focused_semantics_object_id_ = id;
 }
 
+void AccessibilityBridge::AccessibilityObjectDidLoseFocus(int32_t id) {
+  if (last_focused_semantics_object_id_ == id) {
+    last_focused_semantics_object_id_ = kSemanticObjectIdInvalid;
+  }
+}
+
 void AccessibilityBridge::UpdateSemantics(flutter::SemanticsNodeUpdates nodes,
                                           flutter::CustomAccessibilityActionUpdates actions) {
   BOOL layoutChanged = NO;
@@ -198,22 +206,28 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification,
                                                    nextToFocus);
     }
   } else if (layoutChanged) {
-    // Tries to refocus the previous focused semantics object to avoid random jumps.
-    SemanticsObject* nextToFocus =
-        [objects_.get() objectForKey:@(last_focused_semantics_object_id_)];
-    if (!nextToFocus && root) {
-      nextToFocus = FindFirstFocusable(root);
+    SemanticsObject* nextToFocus = nil;
+    // This property will be -1 if the focus is outside of the flutter
+    // application. In this case, we should not refocus anything.
+    if (last_focused_semantics_object_id_ != kSemanticObjectIdInvalid) {
+      // Tries to refocus the previous focused semantics object to avoid random jumps.
+      nextToFocus = [objects_.get() objectForKey:@(last_focused_semantics_object_id_)];
+      if (!nextToFocus && root) {
+        nextToFocus = FindFirstFocusable(root);
+      }
     }
     ios_delegate_->PostAccessibilityNotification(UIAccessibilityLayoutChangedNotification,
                                                  nextToFocus);
   } else if (scrollOccured) {
     // TODO(chunhtai): figure out what string to use for notification. At this
     // point, it is guarantee the previous focused object is still in the tree
     // so that we don't need to worry about focus lost. (e.g. "Screen 0 of 3")
-    SemanticsObject* nextToFocus =
-        [objects_.get() objectForKey:@(last_focused_semantics_object_id_)];
-    if (!nextToFocus && root) {
-      nextToFocus = FindFirstFocusable(root);
+    SemanticsObject* nextToFocus = nil;
+    if (last_focused_semantics_object_id_ != kSemanticObjectIdInvalid) {
+      nextToFocus = [objects_.get() objectForKey:@(last_focused_semantics_object_id_)];
+      if (!nextToFocus && root) {
+        nextToFocus = FindFirstFocusable(root);
+      }
     }
     ios_delegate_->PostAccessibilityNotification(UIAccessibilityPageScrolledNotification,
                                                  nextToFocus);
diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_ios.h b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_ios.h
@@ -25,12 +25,17 @@ class AccessibilityBridgeIos {
                                        flutter::SemanticsAction action,
                                        std::vector<uint8_t> args) = 0;
   /**
-   * A callback that is called after the accessibility focus has moved to a new
-   * SemanticObject.
+   * A callback that is called when a SemanticObject receives focus.
    *
    * The input id is the uid of the newly focused SemanticObject.
    */
-  virtual void AccessibilityFocusDidChange(int32_t id) = 0;
+  virtual void AccessibilityObjectDidBecomeFocused(int32_t id) = 0;
+  /**
+   * A callback that is called when a SemanticObject loses focus
+   *
+   * The input id is the uid of the newly focused SemanticObject.
+   */
+  virtual void AccessibilityObjectDidLoseFocus(int32_t id) = 0;
   virtual FlutterPlatformViewsController* GetPlatformViewsController() const = 0;
 };
 
diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm
@@ -450,7 +450,7 @@ - (void)testAnnouncesLayoutChangeWithNilIfLastFocusIsRemoved {
 
   XCTAssertEqual([accessibility_notifications count], 0ul);
   // Simulates the focusing on the node 1.
-  bridge->AccessibilityFocusDidChange(1);
+  bridge->AccessibilityObjectDidBecomeFocused(1);
 
   flutter::SemanticsNodeUpdates second_update;
   // Simulates the removal of the node 1
@@ -520,7 +520,7 @@ - (void)testAnnouncesLayoutChangeWithLastFocused {
 
   XCTAssertEqual([accessibility_notifications count], 0ul);
   // Simulates the focusing on the node 1.
-  bridge->AccessibilityFocusDidChange(1);
+  bridge->AccessibilityObjectDidBecomeFocused(1);
 
   flutter::SemanticsNodeUpdates second_update;
   // Simulates the removal of the node 2.
@@ -539,6 +539,80 @@ - (void)testAnnouncesLayoutChangeWithLastFocused {
                  UIAccessibilityLayoutChangedNotification);
 }
 
+- (void)testAnnouncesLayoutChangeWhenFocusMovedOutside {
+  flutter::MockDelegate mock_delegate;
+  auto thread_task_runner = CreateNewThread("AccessibilityBridgeTest");
+  flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
+                               /*platform=*/thread_task_runner,
+                               /*raster=*/thread_task_runner,
+                               /*ui=*/thread_task_runner,
+                               /*io=*/thread_task_runner);
+  auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
+      /*delegate=*/mock_delegate,
+      /*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
+      /*task_runners=*/runners);
+  id mockFlutterViewController = OCMClassMock([FlutterViewController class]);
+  id mockFlutterView = OCMClassMock([FlutterView class]);
+  OCMStub([mockFlutterViewController view]).andReturn(mockFlutterView);
+
+  NSMutableArray<NSDictionary<NSString*, id>*>* accessibility_notifications =
+      [[[NSMutableArray alloc] init] autorelease];
+  auto ios_delegate = std::make_unique<flutter::MockIosDelegate>();
+  ios_delegate->on_PostAccessibilityNotification_ =
+      [accessibility_notifications](UIAccessibilityNotifications notification, id argument) {
+        [accessibility_notifications addObject:@{
+          @"notification" : @(notification),
+          @"argument" : argument ? argument : [NSNull null],
+        }];
+      };
+  __block auto bridge =
+      std::make_unique<flutter::AccessibilityBridge>(/*view_controller=*/mockFlutterViewController,
+                                                     /*platform_view=*/platform_view.get(),
+                                                     /*platform_views_controller=*/nil,
+                                                     /*ios_delegate=*/std::move(ios_delegate));
+
+  flutter::CustomAccessibilityActionUpdates actions;
+  flutter::SemanticsNodeUpdates first_update;
+
+  flutter::SemanticsNode node_one;
+  node_one.id = 1;
+  node_one.label = "route1";
+  first_update[node_one.id] = node_one;
+  flutter::SemanticsNode node_two;
+  node_two.id = 2;
+  node_two.label = "route2";
+  first_update[node_two.id] = node_two;
+  flutter::SemanticsNode root_node;
+  root_node.id = kRootNodeId;
+  root_node.label = "root";
+  root_node.childrenInTraversalOrder = {1, 2};
+  root_node.childrenInHitTestOrder = {1, 2};
+  first_update[root_node.id] = root_node;
+  bridge->UpdateSemantics(/*nodes=*/first_update, /*actions=*/actions);
+
+  XCTAssertEqual([accessibility_notifications count], 0ul);
+  // Simulates the focusing on the node 1.
+  bridge->AccessibilityObjectDidBecomeFocused(1);
+  // Simulates that the focus move outside of flutter.
+  bridge->AccessibilityObjectDidLoseFocus(1);
+
+  flutter::SemanticsNodeUpdates second_update;
+  // Simulates the removal of the node 2.
+  flutter::SemanticsNode new_root_node;
+  new_root_node.id = kRootNodeId;
+  new_root_node.label = "root";
+  new_root_node.childrenInTraversalOrder = {1};
+  new_root_node.childrenInHitTestOrder = {1};
+  second_update[root_node.id] = new_root_node;
+  bridge->UpdateSemantics(/*nodes=*/second_update, /*actions=*/actions);
+  NSNull* focusObject = accessibility_notifications[0][@"argument"];
+  // Since the focus is moved outside of the app right before the layout
+  // changed, the bridge should not try to refocus anything .
+  XCTAssertEqual(focusObject, [NSNull null]);
+  XCTAssertEqual([accessibility_notifications[0][@"notification"] unsignedIntValue],
+                 UIAccessibilityLayoutChangedNotification);
+}
+
 - (void)testAnnouncesScrollChangeWithLastFocused {
   flutter::MockDelegate mock_delegate;
   auto thread_task_runner = CreateNewThread("AccessibilityBridgeTest");
@@ -591,7 +665,7 @@ - (void)testAnnouncesScrollChangeWithLastFocused {
   [accessibility_notifications removeAllObjects];
 
   // Simulates the focusing on the node 1.
-  bridge->AccessibilityFocusDidChange(1);
+  bridge->AccessibilityObjectDidBecomeFocused(1);
 
   flutter::SemanticsNodeUpdates second_update;
   // Simulates the scrolling on the node 1.

Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,8 @@ void DispatchSemanticsAction(int32_t id,`
`33`	`33`	`SemanticsActionObservation observation(id, action);`
`34`	`34`	`observations.push_back(observation);`
`35`	`35`	`}`
`36`		`- void AccessibilityFocusDidChange(int32_t id) override {}`
	`36`	`+ void AccessibilityObjectDidBecomeFocused(int32_t id) override {}`
	`37`	`+ void AccessibilityObjectDidLoseFocus(int32_t id) override {}`
`37`	`38`	`FlutterPlatformViewsController* GetPlatformViewsController() const override { return nil; }`
`38`	`39`	`std::vector<SemanticsActionObservation> observations;`
`39`	`40`