Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix editing delta is wrong when using delete key #36616

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 18 additions & 5 deletions lib/web_ui/lib/src/engine/text_editing/text_editing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,12 @@ class TextEditingDeltaState {
/// Infers the correct delta values based on information from the new editing state
/// and the last editing state.
///
/// For a deletion we calculate the length of the deleted text by comparing the new
/// and last editing states. We subtract this from the [deltaEnd] that we set when beforeinput
/// was fired to determine the [deltaStart].
/// For a deletion, the length and the direction of the deletion (backward or forward)
/// are calculated by comparing the new and last editing states.
/// If the deletion is backward, the length is susbtracted from the [deltaEnd]
/// that we set when beforeinput was fired to determine the [deltaStart].
/// If the deletion is forward, [deltaStart] is set to the new editing state baseOffset
/// and [deltaEnd] is set to [deltaStart] incremented by the length of the deletion.
///
/// For a replacement at a selection we set the [deltaStart] to be the beginning of the selection
/// from the last editing state.
Expand All @@ -495,9 +498,19 @@ class TextEditingDeltaState {
if (isTextBeingRemoved) {
// When text is deleted outside of the composing region or is cut using the native toolbar,
// we calculate the length of the deleted text by comparing the new and old editing state lengths.
// This value is then subtracted from the end position of the delta to capture the deleted range.
// If the deletion is backward, the length is susbtracted from the [deltaEnd]
// that we set when beforeinput was fired to determine the [deltaStart].
// If the deletion is forward, [deltaStart] is set to the new editing state baseOffset
// and [deltaEnd] is set to [deltaStart] incremented by the length of the deletion.
final int deletedLength = newTextEditingDeltaState.oldText.length - newEditingState.text!.length;
newTextEditingDeltaState.deltaStart = newTextEditingDeltaState.deltaEnd - deletedLength;
final bool backwardDeletion = newEditingState.baseOffset != lastEditingState?.baseOffset;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems fine to me, though I wonder if RTL affects this at all.

if (backwardDeletion) {
newTextEditingDeltaState.deltaStart = newTextEditingDeltaState.deltaEnd - deletedLength;
} else {
// Forward deletion
newTextEditingDeltaState.deltaStart = newEditingState.baseOffset!;
newTextEditingDeltaState.deltaEnd = newTextEditingDeltaState.deltaStart + deletedLength;
}
} else if (isTextBeingChangedAtActiveSelection) {
// When a selection of text is replaced by a copy/paste operation we set the starting range
// of the delta to be the beginning of the selection of the previous editing state.
Expand Down
62 changes: 60 additions & 2 deletions lib/web_ui/test/text_editing_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2349,10 +2349,18 @@ Future<void> testMain() async {
expect(textEditingDeltaState.composingExtent, -1);
});

test('Verify correct delta is inferred - deletion', () {
test('Verify correct delta is inferred - Backward deletion - Empty selection', () {
final EditingState newEditState = EditingState(text: 'worl', baseOffset: 4, extentOffset: 4);
final EditingState lastEditState = EditingState(text: 'world', baseOffset: 5, extentOffset: 5);
final TextEditingDeltaState deltaState = TextEditingDeltaState(oldText: 'world', deltaStart: 4, deltaEnd: 5, baseOffset: -1, extentOffset: -1, composingOffset: -1, composingExtent: -1);
// `deltaState.deltaEnd` is initialized accordingly to what is done in `DefaultTextEditingStrategy.handleBeforeInput`
final TextEditingDeltaState deltaState = TextEditingDeltaState(
oldText: 'world',
deltaEnd: 5,
baseOffset: -1,
extentOffset: -1,
composingOffset: -1,
composingExtent: -1,
);

final TextEditingDeltaState textEditingDeltaState = TextEditingDeltaState.inferDeltaState(newEditState, lastEditState, deltaState);

Expand All @@ -2366,6 +2374,56 @@ Future<void> testMain() async {
expect(textEditingDeltaState.composingExtent, -1);
});

test('Verify correct delta is inferred - Forward deletion - Empty selection', () {
final EditingState newEditState = EditingState(text: 'worl', baseOffset: 4, extentOffset: 4);
final EditingState lastEditState = EditingState(text: 'world', baseOffset: 4, extentOffset: 4);
// `deltaState.deltaEnd` is initialized accordingly to what is done in `DefaultTextEditingStrategy.handleBeforeInput`
final TextEditingDeltaState deltaState = TextEditingDeltaState(
oldText: 'world',
deltaEnd: 4,
baseOffset: -1,
extentOffset: -1,
composingOffset: -1,
composingExtent: -1,
);

final TextEditingDeltaState textEditingDeltaState = TextEditingDeltaState.inferDeltaState(newEditState, lastEditState, deltaState);

expect(textEditingDeltaState.oldText, 'world');
expect(textEditingDeltaState.deltaText, '');
expect(textEditingDeltaState.deltaStart, 4);
expect(textEditingDeltaState.deltaEnd, 5);
expect(textEditingDeltaState.baseOffset, 4);
expect(textEditingDeltaState.extentOffset, 4);
expect(textEditingDeltaState.composingOffset, -1);
expect(textEditingDeltaState.composingExtent, -1);
});

test('Verify correct delta is inferred - Deletion - Non-empty selection', () {
final EditingState newEditState = EditingState(text: 'w', baseOffset: 1, extentOffset: 1);
final EditingState lastEditState = EditingState(text: 'world', baseOffset: 1, extentOffset: 5);
// `deltaState.deltaEnd` is initialized accordingly to what is done in `DefaultTextEditingStrategy.handleBeforeInput`
final TextEditingDeltaState deltaState = TextEditingDeltaState(
oldText: 'world',
deltaEnd: 5,
baseOffset: -1,
extentOffset: -1,
composingOffset: -1,
composingExtent: -1,
);

final TextEditingDeltaState textEditingDeltaState = TextEditingDeltaState.inferDeltaState(newEditState, lastEditState, deltaState);

expect(textEditingDeltaState.oldText, 'world');
expect(textEditingDeltaState.deltaText, '');
expect(textEditingDeltaState.deltaStart, 1);
expect(textEditingDeltaState.deltaEnd, 5);
expect(textEditingDeltaState.baseOffset, 1);
expect(textEditingDeltaState.extentOffset, 1);
expect(textEditingDeltaState.composingOffset, -1);
expect(textEditingDeltaState.composingExtent, -1);
});

test('Verify correct delta is inferred - composing region replacement', () {
final EditingState newEditState = EditingState(text: '你好吗', baseOffset: 3, extentOffset: 3);
final EditingState lastEditState = EditingState(text: 'ni hao ma', baseOffset: 9, extentOffset: 9);
Expand Down