From 489b1869042234eae7996a4c8a4879dfe166a540 Mon Sep 17 00:00:00 2001 From: Ryan Ferrell Date: Tue, 28 Jan 2025 04:10:19 +0900 Subject: [PATCH] Avoid iOS text selection crash by returning nil range (#161996) [Previously approved](https://github.com/flutter/engine/pull/55909) in /engine before monorepo shift. Reverts an unnecessary assertion introduced by [#16496](https://github.com/flutter/engine/pull/16496) that crashes upon valid user text field interactions. Follow-on work can resolve the underlying discrepancy between iOS and Flutter range finding (see #160100). For crash reports, see issue [#138464](https://github.com/flutter/flutter/issues/138464). ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md Co-authored-by: Chris Bracken --- .../ios/framework/Source/FlutterTextInputPlugin.mm | 9 ++++++++- .../framework/Source/FlutterTextInputPluginTest.mm | 13 +++++++++++++ .../framework/Source/TextInputSemanticsObject.mm | 9 ++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm index 66d2d722ec..a287a54f8b 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm @@ -1293,7 +1293,14 @@ static BOOL IsSelectionRectBoundaryCloserToPoint(CGPoint point, NSAssert([range isKindOfClass:[FlutterTextRange class]], @"Expected a FlutterTextRange for range (got %@).", [range class]); NSRange textRange = ((FlutterTextRange*)range).range; - NSAssert(textRange.location != NSNotFound, @"Expected a valid text range."); + if (textRange.location == NSNotFound) { + // Avoids [crashes](https://github.com/flutter/flutter/issues/138464) from an assertion + // against NSNotFound. + // TODO(hellohuanlin): This is a temp workaround, but we should look into why + // framework is providing NSNotFound to the engine. + // https://github.com/flutter/flutter/issues/160100 + return nil; + } // Sanitize the range to prevent going out of bounds. NSUInteger location = MIN(textRange.location, self.text.length); NSUInteger length = MIN(self.text.length - location, textRange.length); diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm index 5c14a6cfd6..f12b2aa296 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm @@ -530,6 +530,19 @@ FLUTTER_ASSERT_ARC XCTAssertEqual(substring.length, 0ul); } +- (void)testTextInRangeAcceptsNSNotFoundLocationGracefully { + NSDictionary* config = self.mutableTemplateCopy; + [self setClientId:123 configuration:config]; + NSArray* inputFields = self.installedInputViews; + FlutterTextInputView* inputView = inputFields[0]; + + [inputView insertText:@"text"]; + UITextRange* range = [FlutterTextRange rangeWithNSRange:NSMakeRange(NSNotFound, 0)]; + + NSString* substring = [inputView textInRange:range]; + XCTAssertNil(substring); +} + - (void)testStandardEditActions { NSDictionary* config = self.mutableTemplateCopy; [self setClientId:123 configuration:config]; diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/TextInputSemanticsObject.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/TextInputSemanticsObject.mm index f258367ae7..0057d05e0e 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/TextInputSemanticsObject.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/TextInputSemanticsObject.mm @@ -42,7 +42,14 @@ static const UIAccessibilityTraits kUIAccessibilityTraitUndocumentedEmptyLine = NSAssert([range isKindOfClass:[FlutterTextRange class]], @"Expected a FlutterTextRange for range (got %@).", [range class]); NSRange textRange = ((FlutterTextRange*)range).range; - NSAssert(textRange.location != NSNotFound, @"Expected a valid text range."); + if (textRange.location == NSNotFound) { + // Avoids [crashes](https://github.com/flutter/flutter/issues/138464) from an assertion + // against NSNotFound. + // TODO(hellohuanlin): This is a temp workaround, but we should look into why + // framework is providing NSNotFound to the engine. + // https://github.com/flutter/flutter/issues/160100 + return nil; + } return [self.text substringWithRange:textRange]; }