Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[iOS TextInputPlugin] adjust the markedTextRange when replacing text. #46603

Merged
auto-submit merged 5 commits into from
Oct 18, 2023
Merged

[iOS TextInputPlugin] adjust the markedTextRange when replacing text. #46603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8471501
Adjust the markedTextRange when replacing text.
LongCatIsLooong Oct 6, 2023
12a3d68
Fix tests
LongCatIsLooong Oct 6, 2023
62ae972
add a test
LongCatIsLooong Oct 9, 2023
97f0a33
Merge remote-tracking branch 'upstream/main' into ios-adjust-markedTe…
LongCatIsLooong Oct 17, 2023
dbebed5
review
LongCatIsLooong Oct 17, 2023
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
1 change: 0 additions & 1 deletion shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ FLUTTER_DARWIN_EXPORT

// UITextInput
@property(nonatomic, readonly) NSMutableString* text;
@property(nonatomic, readonly) NSMutableString* markedText;
@property(readwrite, copy) UITextRange* selectedTextRange;
@property(nonatomic, strong) UITextRange* markedTextRange;
@property(nonatomic, copy) NSDictionary* markedTextStyle;
Expand Down
66 changes: 27 additions & 39 deletions shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,6 @@ - (instancetype)initWithOwner:(FlutterTextInputPlugin*)textInputPlugin {

// UITextInput
_text = [[NSMutableString alloc] init];
_markedText = [[NSMutableString alloc] init];
_selectedTextRange = [[FlutterTextRange alloc] initWithNSRange:NSMakeRange(0, 0)];
_markedRect = kInvalidFirstRect;
_cachedFirstRect = kInvalidFirstRect;
Expand Down Expand Up @@ -1249,25 +1248,17 @@ - (NSString*)textInRange:(UITextRange*)range {
// Replace the text within the specified range with the given text,
// without notifying the framework.
- (void)replaceRangeLocal:(NSRange)range withText:(NSString*)text {
NSRange selectedRange = _selectedTextRange.range;

// Adjust the text selection:
// * reduce the length by the intersection length
// * adjust the location by newLength - oldLength + intersectionLength
NSRange intersectionRange = NSIntersectionRange(range, selectedRange);
if (range.location <= selectedRange.location) {
selectedRange.location += text.length - range.length;
}
if (intersectionRange.location != NSNotFound) {
selectedRange.location += intersectionRange.length;
selectedRange.length -= intersectionRange.length;
}

[self.text replaceCharactersInRange:[self clampSelection:range forText:self.text]
withString:text];
[self setSelectedTextRangeLocal:[FlutterTextRange
rangeWithNSRange:[self clampSelection:selectedRange
forText:self.text]]];

// Adjust the selected range and the marked text range. There's no
// documentation but UITextField always sets markedTextRange to nil,
// and collapses the selection to the end of the new replacement text.
const NSRange newSelectionRange =
[self clampSelection:NSMakeRange(range.location + text.length, 0) forText:self.text];

[self setSelectedTextRangeLocal:[FlutterTextRange rangeWithNSRange:newSelectionRange]];
self.markedTextRange = nil;
}

- (void)replaceRange:(UITextRange*)range withText:(NSString*)text {
Expand Down Expand Up @@ -1345,11 +1336,10 @@ - (BOOL)shouldChangeTextInRange:(UITextRange*)range replacementText:(NSString*)t
return YES;
}

// Either replaces the existing marked text or, if none is present, inserts it in
// place of the current selection.
- (void)setMarkedText:(NSString*)markedText selectedRange:(NSRange)markedSelectedRange {
NSString* textBeforeChange = [self.text copy];
NSRange selectedRange = _selectedTextRange.range;
NSRange markedTextRange = ((FlutterTextRange*)self.markedTextRange).range;
NSRange actualReplacedRange;

if (_scribbleInteractionStatus != FlutterScribbleInteractionStatusNone ||
_scribbleFocusStatus != FlutterScribbleFocusStatusUnfocused) {
Expand All @@ -1360,26 +1350,24 @@ - (void)setMarkedText:(NSString*)markedText selectedRange:(NSRange)markedSelecte
markedText = @"";
}

if (markedTextRange.length > 0) {
// Replace text in the marked range with the new text.
[self replaceRangeLocal:markedTextRange withText:markedText];
actualReplacedRange = markedTextRange;
markedTextRange.length = markedText.length;
} else {
// Replace text in the selected range with the new text.
actualReplacedRange = selectedRange;
[self replaceRangeLocal:selectedRange withText:markedText];
markedTextRange = NSMakeRange(selectedRange.location, markedText.length);
}
const FlutterTextRange* currentMarkedTextRange = (FlutterTextRange*)self.markedTextRange;
const NSRange& actualReplacedRange = currentMarkedTextRange && !currentMarkedTextRange.isEmpty
? currentMarkedTextRange.range
: _selectedTextRange.range;
// No need to call replaceRangeLocal as this method always adjusts the
// selected/marked text ranges anyways.
Copy link
Contributor

@hellohuanlin hellohuanlin Oct 13, 2023

Choose a reason for hiding this comment

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

I am a bit lost here. Can you explain more why we had this before, and we removed it now?

Copy link
Contributor Author

@LongCatIsLooong LongCatIsLooong Oct 17, 2023

Choose a reason for hiding this comment

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

replaceRangeLocal also changes the selected text range and the marked text range, based on the original selected text range and the marked text range and the text replacement. It's not needed in this method because with this method the caller always have to specify a new selected range and marked range anyways.

Copy link
Contributor

@hellohuanlin hellohuanlin Oct 17, 2023

Choose a reason for hiding this comment

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

Is the the same reason we removed the markedText state?

Copy link
Contributor Author

@LongCatIsLooong LongCatIsLooong Oct 17, 2023

Choose a reason for hiding this comment

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

markedText was never used.

[self.text replaceCharactersInRange:actualReplacedRange withString:markedText];

const NSRange newMarkedRange = NSMakeRange(actualReplacedRange.location, markedText.length);
self.markedTextRange =
markedTextRange.length > 0 ? [FlutterTextRange rangeWithNSRange:markedTextRange] : nil;

NSUInteger selectionLocation = markedSelectedRange.location + markedTextRange.location;
selectedRange = NSMakeRange(selectionLocation, markedSelectedRange.length);
[self setSelectedTextRangeLocal:[FlutterTextRange
rangeWithNSRange:[self clampSelection:selectedRange
forText:self.text]]];
newMarkedRange.length > 0 ? [FlutterTextRange rangeWithNSRange:newMarkedRange] : nil;

[self setSelectedTextRangeLocal:
[FlutterTextRange
rangeWithNSRange:[self clampSelection:NSMakeRange(markedSelectedRange.location +
newMarkedRange.location,
markedSelectedRange.length)
forText:self.text]]];
if (_enableDeltaModel) {
NSRange nextReplaceRange = [self clampSelection:actualReplacedRange forText:textBeforeChange];
[self updateEditingStateWithDelta:flutter::TextEditingDelta(
Expand Down
19 changes: 19 additions & 0 deletions shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,25 @@ - (void)testDisablingAutocorrectDisablesSpellChecking {
XCTAssertEqual(inputView.spellCheckingType, UITextSpellCheckingTypeNo);
}

- (void)testReplaceTestLocalAdjustSelectionAndMarkedTextRange {
FlutterTextInputView* inputView = [[FlutterTextInputView alloc] initWithOwner:textInputPlugin];
[inputView setMarkedText:@"test text" selectedRange:NSMakeRange(0, 5)];
NSRange selectedTextRange = ((FlutterTextRange*)inputView.selectedTextRange).range;
const NSRange markedTextRange = ((FlutterTextRange*)inputView.markedTextRange).range;
XCTAssertEqual(selectedTextRange.location, 0ul);
XCTAssertEqual(selectedTextRange.length, 5ul);
XCTAssertEqual(markedTextRange.location, 0ul);
XCTAssertEqual(markedTextRange.length, 9ul);

// Replaces space with space.
[inputView replaceRange:[FlutterTextRange rangeWithNSRange:NSMakeRange(4, 1)] withText:@" "];
selectedTextRange = ((FlutterTextRange*)inputView.selectedTextRange).range;

XCTAssertEqual(selectedTextRange.location, 5ul);
XCTAssertEqual(selectedTextRange.length, 0ul);
XCTAssertEqual(inputView.markedTextRange, nil);
}

- (void)testFlutterTextInputViewOnlyRespondsToInsertionPointColorBelowIOS17 {
FlutterTextInputView* inputView = [[FlutterTextInputView alloc] initWithOwner:textInputPlugin];
BOOL respondsToInsertionPointColor =
Expand Down