Skip to content

F
Front-End
Unverified Commit b66a5474 authored by Bruno Leroux's avatar Bruno Leroux Committed by GitHub
Browse files
Options

Fix _TextEditingHistoryState reentrant calls (#120889) 

Fix text history undo/redo should not add a new entry to the history
parent d3044a6e
Show whitespace changes
Inline Side-by-side
Showing with 249 additions and 187 deletions
packages/flutter/lib/src/widgets/editable_text.dart
View file @ b66a5474
...@@ -5285,6 +5285,10 @@ class _TextEditingHistoryState extends State<_TextEditingHistory> { ...@@ -5285,6 +5285,10 @@ class _TextEditingHistoryState extends State<_TextEditingHistory> {
late final _Throttled<TextEditingValue> _throttledPush; late final _Throttled<TextEditingValue> _throttledPush;
Timer? _throttleTimer; Timer? _throttleTimer;
// This is used to prevent a reentrant call to the history (a call to _undo or _redo
// should not call _push to add a new entry in the history).
bool _locked = false;
// This duration was chosen as a best fit for the behavior of Mac, Linux, // This duration was chosen as a best fit for the behavior of Mac, Linux,
// and Windows undo/redo state save durations, but it is not perfect for any // and Windows undo/redo state save durations, but it is not perfect for any
// of them. // of them.
...@@ -5305,13 +5309,19 @@ class _TextEditingHistoryState extends State<_TextEditingHistory> { ...@@ -5305,13 +5309,19 @@ class _TextEditingHistoryState extends State<_TextEditingHistory> {
if (nextValue.text == widget.controller.text) { if (nextValue.text == widget.controller.text) {
return; return;
} }
_locked = true;
widget.onTriggered(widget.controller.value.copyWith( widget.onTriggered(widget.controller.value.copyWith(
text: nextValue.text, text: nextValue.text,
selection: nextValue.selection, selection: nextValue.selection,
)); ));
_locked = false;
} }
void _push() { void _push() {
// Do not try to push a new state when the change is related to an undo or redo.
if (_locked) {
return;
}
if (widget.controller.value == TextEditingValue.empty) { if (widget.controller.value == TextEditingValue.empty) {
return; return;
} }
......
packages/flutter/test/widgets/editable_text_test.dart
View file @ b66a5474
...@@ -12773,230 +12773,281 @@ testWidgets('Floating cursor ending with selection', (WidgetTester tester) async ...@@ -12773,230 +12773,281 @@ testWidgets('Floating cursor ending with selection', (WidgetTester tester) async
Future<void> sendUndo(WidgetTester tester) => sendUndoRedo(tester); Future<void> sendUndo(WidgetTester tester) => sendUndoRedo(tester);
Future<void> sendRedo(WidgetTester tester) => sendUndoRedo(tester, true); Future<void> sendRedo(WidgetTester tester) => sendUndoRedo(tester, true);
testWidgets('inside EditableText', (WidgetTester tester) async { Widget boilerplate(TextEditingController controller, [FocusNode? focusNode]) {
final TextEditingController controller = TextEditingController(); return MaterialApp(
final FocusNode focusNode = FocusNode();
await tester.pumpWidget(
MaterialApp(
home: EditableText( home: EditableText(
controller: controller, controller: controller,
focusNode: focusNode, focusNode: focusNode ?? FocusNode(),
style: textStyle, style: textStyle,
cursorColor: Colors.blue, cursorColor: Colors.blue,
backgroundCursorColor: Colors.grey, backgroundCursorColor: Colors.grey,
cursorOpacityAnimates: true, cursorOpacityAnimates: true,
autofillHints: null, autofillHints: null,
), ),
),
); );
}
expect( // Wait for the throttling. This is used to ensure a new history entry is created.
controller.value, Future<void> waitForThrottling(WidgetTester tester) async {
TextEditingValue.empty, await tester.pump(const Duration(milliseconds: 500));
); }
// Undo/redo have no effect on an empty field that has never been edited. // Empty text editing value with a collapsed selection.
await sendUndo(tester); const TextEditingValue emptyTextCollapsed = TextEditingValue(
expect( selection: TextSelection.collapsed(offset: 0),
controller.value,
TextEditingValue.empty,
); );
await sendRedo(tester);
expect( // Texts and text editing values used repeatedly in undo/redo tests.
controller.value, const String textA = 'A';
TextEditingValue.empty, const String textAB = 'AB';
const String textAC = 'AC';
const TextEditingValue textACollapsedAtEnd = TextEditingValue(
text: textA,
selection: TextSelection.collapsed(offset: textA.length),
); );
await tester.pump(); const TextEditingValue textABCollapsedAtEnd = TextEditingValue(
expect( text: textAB,
controller.value, selection: TextSelection.collapsed(offset: textAB.length),
TextEditingValue.empty,
); );
focusNode.requestFocus(); const TextEditingValue textACCollapsedAtEnd = TextEditingValue(
expect( text: textAC,
controller.value, selection: TextSelection.collapsed(offset: textAC.length),
TextEditingValue.empty,
); );
testWidgets('Should have no effect on an empty and non-focused field', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController();
await tester.pumpWidget(boilerplate(controller));
expect(controller.value, TextEditingValue.empty);
// Undo/redo have no effect on an empty field that has never been edited.
await sendUndo(tester);
expect(controller.value, TextEditingValue.empty);
await sendRedo(tester);
expect(controller.value, TextEditingValue.empty);
await tester.pump(); await tester.pump();
expect( expect(controller.value, TextEditingValue.empty);
controller.value,
const TextEditingValue(
selection: TextSelection.collapsed(offset: 0),
),
);
// Wait for the throttling. // On web, these keyboard shortcuts are handled by the browser.
await tester.pump(const Duration(milliseconds: 500)); }, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended]
testWidgets('Should have no effect on an empty and focused field', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController();
final FocusNode focusNode = FocusNode();
await tester.pumpWidget(boilerplate(controller, focusNode));
await waitForThrottling(tester);
expect(controller.value, TextEditingValue.empty);
// Focus the field and wait for throttling delay to get the initial
// state saved in text editing history.
focusNode.requestFocus();
await tester.pump();
expect(controller.value, emptyTextCollapsed);
await waitForThrottling(tester);
// Undo/redo still have no effect. The field is focused and the value has // Undo/redo should have no effect. The field is focused and the value has
// changed, but the text remains empty. // changed, but the text remains empty.
await sendUndo(tester); await sendUndo(tester);
expect( expect(controller.value, emptyTextCollapsed);
controller.value,
const TextEditingValue(
selection: TextSelection.collapsed(offset: 0),
),
);
await sendRedo(tester); await sendRedo(tester);
expect( expect(controller.value, emptyTextCollapsed);
controller.value,
const TextEditingValue(
selection: TextSelection.collapsed(offset: 0),
),
);
await tester.enterText(find.byType(EditableText), '1'); // On web, these keyboard shortcuts are handled by the browser.
expect( }, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended]
controller.value,
const TextEditingValue(
text: '1',
selection: TextSelection.collapsed(offset: 1),
),
);
await tester.pump(const Duration(milliseconds: 500));
// Can undo/redo a single insertion. testWidgets('Can undo/redo a single insertion', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController();
final FocusNode focusNode = FocusNode();
await tester.pumpWidget(boilerplate(controller, focusNode));
// Focus the field and wait for throttling delay to get the initial
// state saved in text editing history.
focusNode.requestFocus();
await tester.pump();
await waitForThrottling(tester);
expect(controller.value, emptyTextCollapsed);
// First insertion.
await tester.enterText(find.byType(EditableText), textA);
await waitForThrottling(tester);
expect(controller.value, textACollapsedAtEnd);
// A redo before any undo has no effect.
await sendRedo(tester);
expect(controller.value, textACollapsedAtEnd);
// Can undo a single insertion.
await sendUndo(tester); await sendUndo(tester);
expect( expect(controller.value, emptyTextCollapsed);
controller.value,
const TextEditingValue( // A second undo has no effect.
selection: TextSelection.collapsed(offset: 0),
),
);
await sendUndo(tester); await sendUndo(tester);
expect( expect(controller.value, emptyTextCollapsed);
controller.value,
const TextEditingValue(
selection: TextSelection.collapsed(offset: 0),
),
);
// Can redo a single insertion.
await sendRedo(tester); await sendRedo(tester);
expect( expect(controller.value, textACollapsedAtEnd);
controller.value,
const TextEditingValue(
text: '1',
selection: TextSelection.collapsed(offset: 1),
),
);
await sendRedo(tester);
expect(
controller.value,
const TextEditingValue(
text: '1',
selection: TextSelection.collapsed(offset: 1),
),
);
// And can undo/redo multiple insertions. // A second redo has no effect.
await tester.enterText(find.byType(EditableText), '13');
expect(
controller.value,
const TextEditingValue(
text: '13',
selection: TextSelection.collapsed(offset: 2),
),
);
await tester.pump(const Duration(milliseconds: 500));
await sendRedo(tester); await sendRedo(tester);
expect( expect(controller.value, textACollapsedAtEnd);
controller.value,
const TextEditingValue( // On web, these keyboard shortcuts are handled by the browser.
text: '13', }, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended]
selection: TextSelection.collapsed(offset: 2),
), testWidgets('Can undo/redo multiple insertions', (WidgetTester tester) async {
); final TextEditingController controller = TextEditingController();
final FocusNode focusNode = FocusNode();
await tester.pumpWidget(boilerplate(controller, focusNode));
// Focus the field and wait for throttling delay to get the initial
// state saved in text editing history.
focusNode.requestFocus();
await tester.pump();
await waitForThrottling(tester);
expect(controller.value, emptyTextCollapsed);
// First insertion.
await tester.enterText(find.byType(EditableText), textA);
await waitForThrottling(tester);
expect(controller.value, textACollapsedAtEnd);
// Second insertion.
await tester.enterText(find.byType(EditableText), textAB);
await waitForThrottling(tester);
expect(controller.value, textABCollapsedAtEnd);
// Undo the first insertion.
await sendUndo(tester); await sendUndo(tester);
expect( expect(controller.value, textACollapsedAtEnd);
controller.value,
const TextEditingValue( // Undo the second insertion.
text: '1',
selection: TextSelection.collapsed(offset: 1),
),
);
await sendUndo(tester); await sendUndo(tester);
expect( expect(controller.value, emptyTextCollapsed);
controller.value,
const TextEditingValue( // Redo the second insertion.
selection: TextSelection.collapsed(offset: 0),
),
);
await sendRedo(tester); await sendRedo(tester);
expect( expect(controller.value, textACollapsedAtEnd);
controller.value,
const TextEditingValue( // Redo the first insertion.
text: '1',
selection: TextSelection.collapsed(offset: 1),
),
);
await sendRedo(tester); await sendRedo(tester);
expect( expect(controller.value, textABCollapsedAtEnd);
controller.value,
const TextEditingValue( // On web, these keyboard shortcuts are handled by the browser.
text: '13', }, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended]
selection: TextSelection.collapsed(offset: 2),
), // Regression test for https://github.com/flutter/flutter/issues/120794.
// This is only reproducible on Android platform because it is the only
// platform where composing changes are saved in the editing history.
testWidgets('Can undo as intented when adding a delay between undos', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController();
final FocusNode focusNode = FocusNode();
await tester.pumpWidget(boilerplate(controller, focusNode));
// Focus the field and wait for throttling delay to get the initial
// state saved in text editing history.
focusNode.requestFocus();
await tester.pump();
await waitForThrottling(tester);
expect(controller.value, emptyTextCollapsed);
final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText));
const TextEditingValue composingStep1 = TextEditingValue(
text: '1 ni',
composing: TextRange(start: 2, end: 4),
selection: TextSelection.collapsed(offset: 4),
); );
// Can change the middle of the stack timeline. const TextEditingValue composingStep2 = TextEditingValue(
await sendUndo(tester); text: '1 nihao',
expect( composing: TextRange(start: 2, end: 7),
controller.value, selection: TextSelection.collapsed(offset: 7),
const TextEditingValue(
text: '1',
selection: TextSelection.collapsed(offset: 1),
),
); );
await tester.enterText(find.byType(EditableText), '12');
const TextEditingValue composingStep3 = TextEditingValue(
text: '1 你好',
selection: TextSelection.collapsed(offset: 4),
);
// Enter some composing text.
state.userUpdateTextEditingValue(composingStep1, SelectionChangedCause.keyboard);
await waitForThrottling(tester);
state.userUpdateTextEditingValue(composingStep2, SelectionChangedCause.keyboard);
await waitForThrottling(tester);
state.userUpdateTextEditingValue(composingStep3, SelectionChangedCause.keyboard);
await waitForThrottling(tester);
// Undo first insertion.
await sendUndo(tester);
expect(controller.value, composingStep2.copyWith(composing: TextRange.empty));
// Waiting for the throttling beetween undos should have no effect.
await tester.pump(const Duration(milliseconds: 500)); await tester.pump(const Duration(milliseconds: 500));
// Undo second insertion.
await sendUndo(tester);
expect(controller.value, composingStep1.copyWith(composing: TextRange.empty));
// On web, these keyboard shortcuts are handled by the browser.
}, variant: TargetPlatformVariant.only(TargetPlatform.android), skip: kIsWeb); // [intended]
testWidgets('Can make changes in the middle of the history', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController();
final FocusNode focusNode = FocusNode();
await tester.pumpWidget(boilerplate(controller, focusNode));
// Focus the field and wait for throttling delay to get the initial
// state saved in text editing history.
focusNode.requestFocus();
await tester.pump();
await waitForThrottling(tester);
expect(controller.value, emptyTextCollapsed);
// First insertion.
await tester.enterText(find.byType(EditableText), textA);
await waitForThrottling(tester);
expect(controller.value, textACollapsedAtEnd);
// Second insertion.
await tester.enterText(find.byType(EditableText), textAC);
await waitForThrottling(tester);
expect(controller.value, textACCollapsedAtEnd);
// Undo and make a change.
await sendUndo(tester);
expect(controller.value, textACollapsedAtEnd);
await tester.enterText(find.byType(EditableText), textAB);
await waitForThrottling(tester);
expect(controller.value, textABCollapsedAtEnd);
// Try a redo, state should not change because of the previous undo.
await sendRedo(tester); await sendRedo(tester);
expect( expect(controller.value, textABCollapsedAtEnd);
controller.value,
const TextEditingValue( // Trying again will have no effect.
text: '12',
selection: TextSelection.collapsed(offset: 2),
),
);
await sendRedo(tester); await sendRedo(tester);
expect( expect(controller.value, textABCollapsedAtEnd);
controller.value,
const TextEditingValue( // Undo should restore state as it was before second insertion.
text: '12',
selection: TextSelection.collapsed(offset: 2),
),
);
await sendUndo(tester); await sendUndo(tester);
expect( expect(controller.value, textACollapsedAtEnd);
controller.value,
const TextEditingValue( // Another undo will restore state as before first insertion.
text: '1',
selection: TextSelection.collapsed(offset: 1),
),
);
await sendUndo(tester); await sendUndo(tester);
expect( expect(controller.value, emptyTextCollapsed);
controller.value,
const TextEditingValue( // Redo all changes.
selection: TextSelection.collapsed(offset: 0),
),
);
await sendRedo(tester); await sendRedo(tester);
expect( expect(controller.value, textACollapsedAtEnd);
controller.value,
const TextEditingValue(
text: '1',
selection: TextSelection.collapsed(offset: 1),
),
);
await sendRedo(tester); await sendRedo(tester);
expect( expect(controller.value, textABCollapsedAtEnd);
controller.value,
const TextEditingValue(
text: '12',
selection: TextSelection.collapsed(offset: 2),
),
);
// On web, these keyboard shortcuts are handled by the browser. // On web, these keyboard shortcuts are handled by the browser.
}, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended] }, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended]
...@@ -13465,6 +13516,7 @@ testWidgets('Floating cursor ending with selection', (WidgetTester tester) async ...@@ -13465,6 +13516,7 @@ testWidgets('Floating cursor ending with selection', (WidgetTester tester) async
selection: TextSelection.collapsed(offset: 7), selection: TextSelection.collapsed(offset: 7),
), ),
); );
await sendUndo(tester); await sendUndo(tester);
expect( expect(
controller.value, controller.value,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment

HIAST - GitLab