diff --git a/packages/flutter/lib/src/services/text_input.dart b/packages/flutter/lib/src/services/text_input.dart index c98f5a22a7..b273db76e0 100644 --- a/packages/flutter/lib/src/services/text_input.dart +++ b/packages/flutter/lib/src/services/text_input.dart @@ -756,17 +756,7 @@ abstract class TextSelectionDelegate { /// Gets the current text input. TextEditingValue get textEditingValue; - /// 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. + /// Sets the current text input (replaces the whole line). set textEditingValue(TextEditingValue value); /// Hides the text selection toolbar. @@ -794,7 +784,6 @@ 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. @@ -816,9 +805,6 @@ 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. @@ -846,10 +832,7 @@ abstract class TextInputClient { /// /// See also: /// -/// * [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]. +/// * [TextInput.attach] class TextInputConnection { TextInputConnection._(this._client) : assert(_client != null), @@ -906,8 +889,7 @@ 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); @@ -1060,57 +1042,9 @@ RawFloatingCursorPoint _toTextPoint(FloatingCursorDragState state, Map? inputFormatters; @@ -1652,66 +1637,61 @@ class EditableTextState extends State with AutomaticKeepAliveClien _clipboardStatus?.removeListener(_onChangedClipboardStatus); _clipboardStatus?.dispose(); super.dispose(); - assert(_batchEditDepth <= 0, 'unfinished batch edits: $_batchEditDepth'); } // TextInputClient implementation: - /// 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; + // _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; @override TextEditingValue get currentTextEditingValue => _value; + bool _updateEditingValueInProgress = false; + @override void updateEditingValue(TextEditingValue value) { - // 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]. - + _updateEditingValueInProgress = true; // 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); } - _lastKnownRemoteTextEditingValue = value; + _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; + } + } 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; - } - - if (value.text == _value.text && value.composing == _value.composing) { + } else if (value.text == _value.text && value.composing == _value.composing && value.selection != _value.selection) { // `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); } @@ -1721,6 +1701,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien _stopCursorTimer(resetCharTicks: false); _startCursorTimer(); } + _updateEditingValueInProgress = false; } @override @@ -1878,52 +1859,33 @@ class EditableTextState extends State with AutomaticKeepAliveClien } // Invoke optional callback with the user's submitted content. - 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'), - )); + 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'), + )); + } } } - 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 (_batchEditDepth > 0 || !_hasInputConnection) + if (!_hasInputConnection) return; final TextEditingValue localValue = _value; - if (localValue == _lastKnownRemoteTextEditingValue) + // 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) 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; @@ -1987,7 +1949,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien return RevealedOffset(rect: rect.shift(unitOffset * offsetDelta), offset: targetOffset); } - bool get _hasInputConnection => _textInputConnection?.attached ?? false; + bool get _hasInputConnection => _textInputConnection != null && _textInputConnection!.attached; bool get _needsAutofill => widget.autofillHints?.isNotEmpty ?? false; bool get _shouldBeInAutofillContext => _needsAutofill && currentAutofillScope != null; @@ -1997,6 +1959,7 @@ 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 @@ -2037,7 +2000,8 @@ class EditableTextState extends State with AutomaticKeepAliveClien if (_hasInputConnection) { _textInputConnection!.close(); _textInputConnection = null; - _lastKnownRemoteTextEditingValue = null; + _lastFormattedUnmodifiedTextEditingValue = null; + _receivedRemoteTextEditingValue = null; } } @@ -2055,7 +2019,8 @@ class EditableTextState extends State with AutomaticKeepAliveClien if (_hasInputConnection) { _textInputConnection!.connectionClosedReceived(); _textInputConnection = null; - _lastKnownRemoteTextEditingValue = null; + _lastFormattedUnmodifiedTextEditingValue = null; + _receivedRemoteTextEditingValue = null; _finalizeEditing(TextInputAction.done, shouldUnfocus: true); } } @@ -2119,15 +2084,17 @@ class EditableTextState extends State with AutomaticKeepAliveClien ); _selectionOverlay!.handlesVisible = widget.showSelectionHandles; _selectionOverlay!.showHandles(); - 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'), - )); + 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'), + )); + } } } } @@ -2215,35 +2182,53 @@ class EditableTextState extends State with AutomaticKeepAliveClien _lastBottomViewInset = WidgetsBinding.instance!.window.viewInsets.bottom; } - late final _WhitespaceDirectionalityFormatter _whitespaceFormatter = _WhitespaceDirectionalityFormatter(textDirection: _textDirection); + _WhitespaceDirectionalityFormatter? _whitespaceFormatter; 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 || _value.composing != value.composing; + final bool textChanged = _value.text != value.text; + final bool isRepeat = value == _lastFormattedUnmodifiedTextEditingValue; - if (textChanged) { + // 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) { // 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. - value = widget.inputFormatters?.fold( - value, - (TextEditingValue newValue, TextInputFormatter formatter) => formatter.formatEditUpdate(_value, newValue), - ) ?? value; - + for (final TextInputFormatter formatter in widget.inputFormatters!) { + value = formatter.formatEditUpdate(_value, value); + } // Always pass the text through the whitespace directionality formatter to // maintain expected behavior with carets on trailing whitespace. - value = _whitespaceFormatter.formatEditUpdate(_value, value); + value = _whitespaceFormatter!.formatEditUpdate(_value, value); + _lastFormattedValue = value; } - // Put all optional user callback invocations in a batch edit to prevent - // sending multiple `TextInput.updateEditingValue` messages. - beginBatchEdit(); + 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; + } - _value = value; - if (textChanged) { + // Use the last formatted value when an identical repeat pass is detected. + if (isRepeat && textChanged && _lastFormattedValue != null) { + _value = _lastFormattedValue!; + } + + if (textChanged && widget.onChanged != null) { try { - widget.onChanged?.call(value.text); + widget.onChanged!(value.text); } catch (exception, stack) { FlutterError.reportError(FlutterErrorDetails( exception: exception, @@ -2253,8 +2238,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien )); } } - - endBatchEdit(); + _lastFormattedUnmodifiedTextEditingValue = _receivedRemoteTextEditingValue; } void _onCursorColorTick() { diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index 7491fbc27e..01f8942729 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -4736,246 +4736,6 @@ 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); @@ -5475,12 +5235,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 triggers reformat - expect(formatter.formatCallCount, 4); + 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: 2)); // The new composing was registered in formatter. + 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. - expect(formatter.formatCallCount, 5); + expect(formatter.formatCallCount, 4); expect(formatter.lastOldValue.composing, const TextRange(start: 1, end: 2)); expect(formatter.lastNewValue.composing, const TextRange(start: -1, end: -1)); @@ -5491,10 +5251,8 @@ void main() { '[2]: normal aaaa', '[3]: 012, 0123', '[3]: normal aaaaaa', - '[4]: 0123, 0123', - '[4]: normal aaaaaaaa', - '[5]: 0123, 01234', - '[5]: normal aaaaaaaaaa', + '[4]: 0123, 01234', + '[4]: normal aaaaaaaa' ]; expect(formatter.log, referenceLog);