Skip to content

Commit

Permalink
Reverts part of "fix auto-correction highlight on top left corner (Ag…
Browse files Browse the repository at this point in the history
…ain)" (#45523)

This reverts part of #44779. Verified this to be the regression for flutter/flutter#133908. 

This pair of `textWill/DidChange` calls was added in Beta 1, in order to fix highlight region missing last character (more details [here](https://docs.google.com/document/d/1sM3HMv-SQin39yX1aPUU7vtGv7Hcef1Quc3QhRXBl6A/edit?resourcekey=0-SFYD8vmOIkXiXCZvB1Wlcw#heading=h.ddlvu2i2epyl)). 

However, that fix doesn't work anymore for Beta 7 - it turns out that in Beta 7, the system doesn't rely on the `selectionRects` to determine the highlight region anymore. Instead, the system relies on `firstRectForRange` API. So we fixed the system highlight in `firstRectForRange` in Beta 7. 

So even after reverting this change, the system highlight still works correctly in Beta 7. I was initially leaning towards keeping these calls even after Beta 7, because without it, the `selectionRects` are still out of sync with iOS text input system. (more details [here](https://docs.google.com/document/d/1sM3HMv-SQin39yX1aPUU7vtGv7Hcef1Quc3QhRXBl6A/edit?resourcekey=0-SFYD8vmOIkXiXCZvB1Wlcw)). 

However, I was able to verify that it is indeed `textWill/DidChange` that introduced the regression for IME inputs (It's unclear why they are related, so need further investigation). 

In summary, after this revert, system highlights are still in the right place, and with the right length. 

*List which issues are fixed by this PR. You must list at least one issue.*

Fixes flutter/flutter#133908

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
  • Loading branch information
hellohuanlin committed Sep 7, 2023
1 parent 0e633ce commit 554a456
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2540,21 +2540,10 @@ - (void)setSelectionRects:(NSArray*)encodedRects {
: NSWritingDirectionRightToLeft]];
}

BOOL shouldNotifyTextChange = NO;
if (@available(iOS 17, *)) {
// Force UIKit to query the selectionRects again on iOS 17+
// This is to fix a bug on iOS 17+ where UIKit queries the outdated selectionRects after
// entering a character, resulting in auto-correction highlight region missing the last
// character.
shouldNotifyTextChange = YES;
}
if (shouldNotifyTextChange) {
[_activeView.inputDelegate textWillChange:_activeView];
}
// TODO(hellohuanlin): Investigate why notifying the text input system about text changes (via
// textWillChange and textDidChange APIs) causes a bug where we cannot enter text with IME
// keyboards. Issue: https://github.com/flutter/flutter/issues/133908
_activeView.selectionRects = rectsAsRect;
if (shouldNotifyTextChange) {
[_activeView.inputDelegate textDidChange:_activeView];
}
}

- (void)startLiveTextInput {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,38 +457,6 @@ - (void)testInputHiderOverlapWithTextWhenScribbleIsDisabledAfterIOS17AndDoesNotO
}
}

- (void)testSetSelectionRectsNotifiesTextChangeAfterIOS17AndDoesNotNotifyBeforeIOS17 {
FlutterTextInputPlugin* myInputPlugin =
[[FlutterTextInputPlugin alloc] initWithDelegate:OCMClassMock([FlutterEngine class])];

FlutterMethodCall* setClientCall =
[FlutterMethodCall methodCallWithMethodName:@"TextInput.setClient"
arguments:@[ @(123), self.mutableTemplateCopy ]];
[myInputPlugin handleMethodCall:setClientCall
result:^(id _Nullable result){
}];

id mockInputDelegate = OCMProtocolMock(@protocol(UITextInputDelegate));
myInputPlugin.activeView.inputDelegate = mockInputDelegate;

NSArray<NSNumber*>* selectionRect = [NSArray arrayWithObjects:@0, @0, @100, @100, @0, @1, nil];
NSArray* selectionRects = [NSArray arrayWithObjects:selectionRect, nil];
FlutterMethodCall* methodCall =
[FlutterMethodCall methodCallWithMethodName:@"Scribble.setSelectionRects"
arguments:selectionRects];
[myInputPlugin handleMethodCall:methodCall
result:^(id _Nullable result){
}];

if (@available(iOS 17.0, *)) {
OCMVerify([mockInputDelegate textWillChange:myInputPlugin.activeView]);
OCMVerify([mockInputDelegate textDidChange:myInputPlugin.activeView]);
} else {
OCMVerify(never(), [mockInputDelegate textWillChange:myInputPlugin.activeView]);
OCMVerify(never(), [mockInputDelegate textDidChange:myInputPlugin.activeView]);
}
}

- (void)testTextRangeFromPositionMatchesUITextViewBehavior {
FlutterTextInputView* inputView = [[FlutterTextInputView alloc] initWithOwner:textInputPlugin];
FlutterTextPosition* fromPosition = [FlutterTextPosition positionWithIndex:2];
Expand Down

0 comments on commit 554a456

Please sign in to comment.