diff --git a/packages/flutter/lib/src/services/text_input.dart b/packages/flutter/lib/src/services/text_input.dart index b273db76e08..c98f5a22a79 100644 --- a/packages/flutter/lib/src/services/text_input.dart +++ b/packages/flutter/lib/src/services/text_input.dart @@ -756,7 +756,17 @@ abstract class TextSelectionDelegate { /// Gets the current text input. TextEditingValue get textEditingValue; - /// Sets the current text input (replaces the whole line). + /// Indicates that the user has requested the delegate to replace its current + /// text editing state with [value]. + /// + /// The new [value] is treated as user input and thus may subject to input + /// formatting. + /// + /// See also: + /// + /// * [EditableTextState.textEditingValue]: an implementation that applies + /// additional pre-processing to the specified [value], before updating the + /// text editing state. set textEditingValue(TextEditingValue value); /// Hides the text selection toolbar. @@ -784,6 +794,7 @@ abstract class TextSelectionDelegate { /// See also: /// /// * [TextInput.attach] +/// * [EditableText], a [TextInputClient] implementation. abstract class TextInputClient { /// Abstract const constructor. This constructor enables subclasses to provide /// const constructors so that they can be used in const expressions. @@ -805,6 +816,9 @@ abstract class TextInputClient { AutofillScope? get currentAutofillScope; /// Requests that this client update its editing state to the given value. + /// + /// The new [value] is treated as user input and thus may subject to input + /// formatting. void updateEditingValue(TextEditingValue value); /// Requests that this client perform the given action. @@ -832,7 +846,10 @@ abstract class TextInputClient { /// /// See also: /// -/// * [TextInput.attach] +/// * [TextInput.attach], a method used to establish a [TextInputConnection] +/// between the system's text input and a [TextInputClient]. +/// * [EditableText], a [TextInputClient] that connects to and interacts with +/// the system's text input using a [TextInputConnection]. class TextInputConnection { TextInputConnection._(this._client) : assert(_client != null), @@ -889,7 +906,8 @@ class TextInputConnection { TextInput._instance._updateConfig(configuration); } - /// Requests that the text input control change its internal state to match the given state. + /// Requests that the text input control change its internal state to match + /// the given state. void setEditingState(TextEditingValue value) { assert(attached); TextInput._instance._setEditingState(value); @@ -1042,9 +1060,57 @@ RawFloatingCursorPoint _toTextPoint(FloatingCursorDragState state, Map? inputFormatters; @@ -1637,61 +1652,66 @@ class EditableTextState extends State with AutomaticKeepAliveClien _clipboardStatus?.removeListener(_onChangedClipboardStatus); _clipboardStatus?.dispose(); super.dispose(); + assert(_batchEditDepth <= 0, 'unfinished batch edits: $_batchEditDepth'); } // TextInputClient implementation: - // _lastFormattedUnmodifiedTextEditingValue tracks the last value - // that the formatter ran on and is used to prevent double-formatting. - TextEditingValue? _lastFormattedUnmodifiedTextEditingValue; - // _lastFormattedValue tracks the last post-format value, so that it can be - // reused without rerunning the formatter when the input value is repeated. - TextEditingValue? _lastFormattedValue; - // _receivedRemoteTextEditingValue is the direct value last passed in - // updateEditingValue. This value does not get updated with the formatted - // version. - TextEditingValue? _receivedRemoteTextEditingValue; + /// The last known [TextEditingValue] of the platform text input plugin. + /// + /// This value is updated when the platform text input plugin sends a new + /// update via [updateEditingValue], or when [EditableText] calls + /// [TextInputConnection.setEditingState] to overwrite the platform text input + /// plugin's [TextEditingValue]. + /// + /// Used in [_updateRemoteEditingValueIfNeeded] to determine whether the + /// remote value is outdated and needs updating. + TextEditingValue? _lastKnownRemoteTextEditingValue; @override TextEditingValue get currentTextEditingValue => _value; - bool _updateEditingValueInProgress = false; - @override void updateEditingValue(TextEditingValue value) { - _updateEditingValueInProgress = true; + // This method handles text editing state updates from the platform text + // input plugin. The [EditableText] may not have the focus or an open input + // connection, as autofill can update a disconnected [EditableText]. + // Since we still have to support keyboard select, this is the best place // to disable text updating. if (!_shouldCreateInputConnection) { - _updateEditingValueInProgress = false; return; } + if (widget.readOnly) { // In the read-only case, we only care about selection changes, and reject // everything else. value = _value.copyWith(selection: value.selection); } - _receivedRemoteTextEditingValue = value; - if (value.text != _value.text) { - hideToolbar(); - _showCaretOnScreen(); - _currentPromptRectRange = null; - if (widget.obscureText && value.text.length == _value.text.length + 1) { - _obscureShowCharTicksPending = _kObscureShowLatestCharCursorTicks; - _obscureLatestCharIndex = _value.selection.baseOffset; - } - } + _lastKnownRemoteTextEditingValue = value; if (value == _value) { // This is possible, for example, when the numeric keyboard is input, // the engine will notify twice for the same value. // Track at https://github.com/flutter/flutter/issues/65811 - _updateEditingValueInProgress = false; return; - } else if (value.text == _value.text && value.composing == _value.composing && value.selection != _value.selection) { + } + + if (value.text == _value.text && value.composing == _value.composing) { // `selection` is the only change. _handleSelectionChanged(value.selection, renderEditable, SelectionChangedCause.keyboard); } else { + hideToolbar(); + _currentPromptRectRange = null; + + if (_hasInputConnection) { + _showCaretOnScreen(); + if (widget.obscureText && value.text.length == _value.text.length + 1) { + _obscureShowCharTicksPending = _kObscureShowLatestCharCursorTicks; + _obscureLatestCharIndex = _value.selection.baseOffset; + } + } + _formatAndSetValue(value); } @@ -1701,7 +1721,6 @@ class EditableTextState extends State with AutomaticKeepAliveClien _stopCursorTimer(resetCharTicks: false); _startCursorTimer(); } - _updateEditingValueInProgress = false; } @override @@ -1859,33 +1878,52 @@ class EditableTextState extends State with AutomaticKeepAliveClien } // Invoke optional callback with the user's submitted content. - if (widget.onSubmitted != null) { - try { - widget.onSubmitted!(_value.text); - } catch (exception, stack) { - FlutterError.reportError(FlutterErrorDetails( - exception: exception, - stack: stack, - library: 'widgets', - context: ErrorDescription('while calling onSubmitted for $action'), - )); - } + try { + widget.onSubmitted?.call(_value.text); + } catch (exception, stack) { + FlutterError.reportError(FlutterErrorDetails( + exception: exception, + stack: stack, + library: 'widgets', + context: ErrorDescription('while calling onSubmitted for $action'), + )); } } + int _batchEditDepth = 0; + + /// Begins a new batch edit, within which new updates made to the text editing + /// value will not be sent to the platform text input plugin. + /// + /// Batch edits nest. When the outmost batch edit finishes, [endBatchEdit] + /// will attempt to send [currentTextEditingValue] to the text input plugin if + /// it detected a change. + void beginBatchEdit() { + _batchEditDepth += 1; + } + + /// Ends the current batch edit started by the last call to [beginBatchEdit], + /// and send [currentTextEditingValue] to the text input plugin if needed. + /// + /// Throws an error in debug mode if this [EditableText] is not in a batch + /// edit. + void endBatchEdit() { + _batchEditDepth -= 1; + assert( + _batchEditDepth >= 0, + 'Unbalanced call to endBatchEdit: beginBatchEdit must be called first.', + ); + _updateRemoteEditingValueIfNeeded(); + } + void _updateRemoteEditingValueIfNeeded() { - if (!_hasInputConnection) + if (_batchEditDepth > 0 || !_hasInputConnection) return; final TextEditingValue localValue = _value; - // We should not update back the value notified by the remote(engine) in reverse, this is redundant. - // Unless we modify this value for some reason during processing, such as `TextInputFormatter`. - if (_updateEditingValueInProgress && localValue == _receivedRemoteTextEditingValue) + if (localValue == _lastKnownRemoteTextEditingValue) return; - // In other cases, as long as the value of the [widget.controller.value] is modified, - // `setEditingState` should be called as we do not want to skip sending real changes - // to the engine. - // Also see https://github.com/flutter/flutter/issues/65059#issuecomment-690254379 _textInputConnection!.setEditingState(localValue); + _lastKnownRemoteTextEditingValue = localValue; } TextEditingValue get _value => widget.controller.value; @@ -1949,7 +1987,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien return RevealedOffset(rect: rect.shift(unitOffset * offsetDelta), offset: targetOffset); } - bool get _hasInputConnection => _textInputConnection != null && _textInputConnection!.attached; + bool get _hasInputConnection => _textInputConnection?.attached ?? false; bool get _needsAutofill => widget.autofillHints?.isNotEmpty ?? false; bool get _shouldBeInAutofillContext => _needsAutofill && currentAutofillScope != null; @@ -1959,7 +1997,6 @@ class EditableTextState extends State with AutomaticKeepAliveClien } if (!_hasInputConnection) { final TextEditingValue localValue = _value; - _lastFormattedUnmodifiedTextEditingValue = localValue; // When _needsAutofill == true && currentAutofillScope == null, autofill // is allowed but saving the user input from the text field is @@ -2000,8 +2037,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien if (_hasInputConnection) { _textInputConnection!.close(); _textInputConnection = null; - _lastFormattedUnmodifiedTextEditingValue = null; - _receivedRemoteTextEditingValue = null; + _lastKnownRemoteTextEditingValue = null; } } @@ -2019,8 +2055,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien if (_hasInputConnection) { _textInputConnection!.connectionClosedReceived(); _textInputConnection = null; - _lastFormattedUnmodifiedTextEditingValue = null; - _receivedRemoteTextEditingValue = null; + _lastKnownRemoteTextEditingValue = null; _finalizeEditing(TextInputAction.done, shouldUnfocus: true); } } @@ -2084,17 +2119,15 @@ class EditableTextState extends State with AutomaticKeepAliveClien ); _selectionOverlay!.handlesVisible = widget.showSelectionHandles; _selectionOverlay!.showHandles(); - if (widget.onSelectionChanged != null) { - try { - widget.onSelectionChanged!(selection, cause); - } catch (exception, stack) { - FlutterError.reportError(FlutterErrorDetails( - exception: exception, - stack: stack, - library: 'widgets', - context: ErrorDescription('while calling onSelectionChanged for $cause'), - )); - } + try { + widget.onSelectionChanged?.call(selection, cause); + } catch (exception, stack) { + FlutterError.reportError(FlutterErrorDetails( + exception: exception, + stack: stack, + library: 'widgets', + context: ErrorDescription('while calling onSelectionChanged for $cause'), + )); } } } @@ -2182,53 +2215,35 @@ class EditableTextState extends State with AutomaticKeepAliveClien _lastBottomViewInset = WidgetsBinding.instance!.window.viewInsets.bottom; } - _WhitespaceDirectionalityFormatter? _whitespaceFormatter; + late final _WhitespaceDirectionalityFormatter _whitespaceFormatter = _WhitespaceDirectionalityFormatter(textDirection: _textDirection); void _formatAndSetValue(TextEditingValue value) { - _whitespaceFormatter ??= _WhitespaceDirectionalityFormatter(textDirection: _textDirection); - // Check if the new value is the same as the current local value, or is the same // as the pre-formatting value of the previous pass (repeat call). - final bool textChanged = _value.text != value.text; - final bool isRepeat = value == _lastFormattedUnmodifiedTextEditingValue; + final bool textChanged = _value.text != value.text || _value.composing != value.composing; - // There's no need to format when starting to compose or when continuing - // an existing composition. - final bool isComposing = value.composing.isValid; - final bool isPreviouslyComposing = _lastFormattedUnmodifiedTextEditingValue?.composing.isValid ?? false; - - if ((textChanged || (!isComposing && isPreviouslyComposing)) && - widget.inputFormatters != null && - widget.inputFormatters!.isNotEmpty) { + if (textChanged) { // Only format when the text has changed and there are available formatters. // Pass through the formatter regardless of repeat status if the input value is // different than the stored value. - for (final TextInputFormatter formatter in widget.inputFormatters!) { - value = formatter.formatEditUpdate(_value, value); - } + value = widget.inputFormatters?.fold( + value, + (TextEditingValue newValue, TextInputFormatter formatter) => formatter.formatEditUpdate(_value, newValue), + ) ?? value; + // Always pass the text through the whitespace directionality formatter to // maintain expected behavior with carets on trailing whitespace. - value = _whitespaceFormatter!.formatEditUpdate(_value, value); - _lastFormattedValue = value; + value = _whitespaceFormatter.formatEditUpdate(_value, value); } - if (value == _value) { - // If the value was modified by the formatter, the remote should be notified to keep in sync, - // if not modified, it will short-circuit. - _updateRemoteEditingValueIfNeeded(); - } else { - // Setting _value here ensures the selection and composing region info is passed. - _value = value; - } + // Put all optional user callback invocations in a batch edit to prevent + // sending multiple `TextInput.updateEditingValue` messages. + beginBatchEdit(); - // Use the last formatted value when an identical repeat pass is detected. - if (isRepeat && textChanged && _lastFormattedValue != null) { - _value = _lastFormattedValue!; - } - - if (textChanged && widget.onChanged != null) { + _value = value; + if (textChanged) { try { - widget.onChanged!(value.text); + widget.onChanged?.call(value.text); } catch (exception, stack) { FlutterError.reportError(FlutterErrorDetails( exception: exception, @@ -2238,7 +2253,8 @@ class EditableTextState extends State with AutomaticKeepAliveClien )); } } - _lastFormattedUnmodifiedTextEditingValue = _receivedRemoteTextEditingValue; + + endBatchEdit(); } void _onCursorColorTick() { diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index 01f8942729d..7491fbc27ea 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -4736,6 +4736,246 @@ void main() { ); }); + group('batch editing', () { + final TextEditingController controller = TextEditingController(text: testText); + final EditableText editableText = EditableText( + showSelectionHandles: true, + maxLines: 2, + controller: controller, + focusNode: FocusNode(), + cursorColor: Colors.red, + backgroundCursorColor: Colors.blue, + style: Typography.material2018(platform: TargetPlatform.android).black.subtitle1.copyWith(fontFamily: 'Roboto'), + keyboardType: TextInputType.text, + ); + + final Widget widget = MediaQuery( + data: const MediaQueryData(), + child: Directionality( + textDirection: TextDirection.ltr, + child: editableText, + ), + ); + + testWidgets('batch editing works', (WidgetTester tester) async { + await tester.pumpWidget(widget); + + // Connect. + await tester.showKeyboard(find.byType(EditableText)); + + final EditableTextState state = tester.state(find.byWidget(editableText)); + state.updateEditingValue(const TextEditingValue(text: 'remote value')); + tester.testTextInput.log.clear(); + + state.beginBatchEdit(); + + controller.text = 'new change 1'; + expect(state.currentTextEditingValue.text, 'new change 1'); + expect(tester.testTextInput.log, isEmpty); + + // Nesting. + state.beginBatchEdit(); + controller.text = 'new change 2'; + expect(state.currentTextEditingValue.text, 'new change 2'); + expect(tester.testTextInput.log, isEmpty); + + // End the innermost batch edit. Not yet. + state.endBatchEdit(); + expect(tester.testTextInput.log, isEmpty); + + controller.text = 'new change 3'; + expect(state.currentTextEditingValue.text, 'new change 3'); + expect(tester.testTextInput.log, isEmpty); + + // Finish the outermost batch edit. + state.endBatchEdit(); + expect(tester.testTextInput.log, hasLength(1)); + expect( + tester.testTextInput.log, + contains(matchesMethodCall('TextInput.setEditingState', args: containsPair('text', 'new change 3'))), + ); + }); + + testWidgets('batch edits need to be nested properly', (WidgetTester tester) async { + await tester.pumpWidget(widget); + + // Connect. + await tester.showKeyboard(find.byType(EditableText)); + + final EditableTextState state = tester.state(find.byWidget(editableText)); + state.updateEditingValue(const TextEditingValue(text: 'remote value')); + tester.testTextInput.log.clear(); + + String errorString; + try { + state.endBatchEdit(); + } catch (e) { + errorString = e.toString(); + } + + expect(errorString, contains('Unbalanced call to endBatchEdit')); + }); + + testWidgets('catch unfinished batch edits on disposal', (WidgetTester tester) async { + await tester.pumpWidget(widget); + + // Connect. + await tester.showKeyboard(find.byType(EditableText)); + + final EditableTextState state = tester.state(find.byWidget(editableText)); + state.updateEditingValue(const TextEditingValue(text: 'remote value')); + tester.testTextInput.log.clear(); + + state.beginBatchEdit(); + expect(tester.takeException(), isNull); + + await tester.pumpWidget(Container()); + expect(tester.takeException(), isNotNull); + }); + }); + + group('EditableText does not send editing values more than once', () { + final TextEditingController controller = TextEditingController(text: testText); + final EditableText editableText = EditableText( + showSelectionHandles: true, + maxLines: 2, + controller: controller, + focusNode: FocusNode(), + cursorColor: Colors.red, + backgroundCursorColor: Colors.blue, + style: Typography.material2018(platform: TargetPlatform.android).black.subtitle1.copyWith(fontFamily: 'Roboto'), + keyboardType: TextInputType.text, + inputFormatters: [LengthLimitingTextInputFormatter(6)], + onChanged: (String s) => controller.text += ' onChanged', + ); + + final Widget widget = MediaQuery( + data: const MediaQueryData(), + child: Directionality( + textDirection: TextDirection.ltr, + child: editableText, + ), + ); + + controller.addListener(() { + if (!controller.text.endsWith('listener')) + controller.text += ' listener'; + }); + + testWidgets('input from text input plugin', (WidgetTester tester) async { + await tester.pumpWidget(widget); + + // Connect. + await tester.showKeyboard(find.byType(EditableText)); + tester.testTextInput.log.clear(); + + final EditableTextState state = tester.state(find.byWidget(editableText)); + state.updateEditingValue(const TextEditingValue(text: 'remoteremoteremote')); + + // Apply in order: length formatter -> listener -> onChanged -> listener. + expect(controller.text, 'remote listener onChanged listener'); + final List updates = tester.testTextInput.log + .where((MethodCall call) => call.method == 'TextInput.setEditingState') + .map((MethodCall call) => TextEditingValue.fromJSON(call.arguments as Map)) + .toList(growable: false); + + expect(updates, const [TextEditingValue(text: 'remote listener onChanged listener')]); + + tester.testTextInput.log.clear(); + + // If by coincidence the text input plugin sends the same value back, + // do nothing. + state.updateEditingValue(const TextEditingValue(text: 'remote listener onChanged listener')); + expect(controller.text, 'remote listener onChanged listener'); + expect(tester.testTextInput.log, isEmpty); + }); + + testWidgets('input from text selection menu', (WidgetTester tester) async { + await tester.pumpWidget(widget); + + // Connect. + await tester.showKeyboard(find.byType(EditableText)); + tester.testTextInput.log.clear(); + + final EditableTextState state = tester.state(find.byWidget(editableText)); + state.textEditingValue = const TextEditingValue(text: 'remoteremoteremote'); + + // Apply in order: length formatter -> listener -> onChanged -> listener. + expect(controller.text, 'remote listener onChanged listener'); + final List updates = tester.testTextInput.log + .where((MethodCall call) => call.method == 'TextInput.setEditingState') + .map((MethodCall call) => TextEditingValue.fromJSON(call.arguments as Map)) + .toList(growable: false); + + expect(updates, const [TextEditingValue(text: 'remote listener onChanged listener')]); + + tester.testTextInput.log.clear(); + }); + + testWidgets('input from controller', (WidgetTester tester) async { + await tester.pumpWidget(widget); + + // Connect. + await tester.showKeyboard(find.byType(EditableText)); + tester.testTextInput.log.clear(); + + controller.text = 'remoteremoteremote'; + final List updates = tester.testTextInput.log + .where((MethodCall call) => call.method == 'TextInput.setEditingState') + .map((MethodCall call) => TextEditingValue.fromJSON(call.arguments as Map)) + .toList(growable: false); + + expect(updates, const [TextEditingValue(text: 'remoteremoteremote listener')]); + }); + + testWidgets('input from changing controller', (WidgetTester tester) async { + final TextEditingController controller = TextEditingController(text: testText); + Widget build({ TextEditingController textEditingController }) { + return MediaQuery( + data: const MediaQueryData(), + child: Directionality( + textDirection: TextDirection.ltr, + child: EditableText( + showSelectionHandles: true, + maxLines: 2, + controller: textEditingController ?? controller, + focusNode: FocusNode(), + cursorColor: Colors.red, + backgroundCursorColor: Colors.blue, + style: Typography.material2018(platform: TargetPlatform.android).black.subtitle1.copyWith(fontFamily: 'Roboto'), + keyboardType: TextInputType.text, + inputFormatters: [LengthLimitingTextInputFormatter(6)], + ), + ), + ); + } + + await tester.pumpWidget(build()); + + // Connect. + await tester.showKeyboard(find.byType(EditableText)); + tester.testTextInput.log.clear(); + await tester.pumpWidget(build(textEditingController: TextEditingController(text: 'new text'))); + + List updates = tester.testTextInput.log + .where((MethodCall call) => call.method == 'TextInput.setEditingState') + .map((MethodCall call) => TextEditingValue.fromJSON(call.arguments as Map)) + .toList(growable: false); + + expect(updates, const [TextEditingValue(text: 'new text')]); + + tester.testTextInput.log.clear(); + await tester.pumpWidget(build(textEditingController: TextEditingController(text: 'new new text'))); + + updates = tester.testTextInput.log + .where((MethodCall call) => call.method == 'TextInput.setEditingState') + .map((MethodCall call) => TextEditingValue.fromJSON(call.arguments as Map)) + .toList(growable: false); + + expect(updates, const [TextEditingValue(text: 'new new text')]); + }); + }); + testWidgets('input imm channel calls are ordered correctly', (WidgetTester tester) async { const String testText = 'flutter is the best!'; final TextEditingController controller = TextEditingController(text: testText); @@ -5235,12 +5475,12 @@ void main() { expect(formatter.formatCallCount, 3); state.updateEditingValue(const TextEditingValue(text: '0123', selection: TextSelection.collapsed(offset: 2))); // No text change, does not format expect(formatter.formatCallCount, 3); - state.updateEditingValue(const TextEditingValue(text: '0123', selection: TextSelection.collapsed(offset: 2), composing: TextRange(start: 1, end: 2))); // Composing change does not reformat - expect(formatter.formatCallCount, 3); - expect(formatter.lastOldValue.composing, const TextRange(start: -1, end: -1)); - expect(formatter.lastNewValue.composing, const TextRange(start: -1, end: -1)); // Since did not format, the new composing was not registered in formatter. - state.updateEditingValue(const TextEditingValue(text: '01234', selection: TextSelection.collapsed(offset: 2))); // Formats, with oldValue containing composing region. + state.updateEditingValue(const TextEditingValue(text: '0123', selection: TextSelection.collapsed(offset: 2), composing: TextRange(start: 1, end: 2))); // Composing change triggers reformat expect(formatter.formatCallCount, 4); + expect(formatter.lastOldValue.composing, const TextRange(start: -1, end: -1)); + expect(formatter.lastNewValue.composing, const TextRange(start: 1, end: 2)); // The new composing was registered in formatter. + state.updateEditingValue(const TextEditingValue(text: '01234', selection: TextSelection.collapsed(offset: 2))); // Formats, with oldValue containing composing region. + expect(formatter.formatCallCount, 5); expect(formatter.lastOldValue.composing, const TextRange(start: 1, end: 2)); expect(formatter.lastNewValue.composing, const TextRange(start: -1, end: -1)); @@ -5251,8 +5491,10 @@ void main() { '[2]: normal aaaa', '[3]: 012, 0123', '[3]: normal aaaaaa', - '[4]: 0123, 01234', - '[4]: normal aaaaaaaa' + '[4]: 0123, 0123', + '[4]: normal aaaaaaaa', + '[5]: 0123, 01234', + '[5]: normal aaaaaaaaaa', ]; expect(formatter.log, referenceLog);