From 0d945a1a5643ab9d1091e9bc56aed4b281f05463 Mon Sep 17 00:00:00 2001 From: xubaolin Date: Fri, 18 Sep 2020 06:27:11 +0800 Subject: [PATCH] Fix the inconsistency between the local state of the input and the engine state (#65754) --- .../lib/src/widgets/editable_text.dart | 30 +++-- .../test/widgets/editable_text_test.dart | 126 ++++++++++++++++++ 2 files changed, 146 insertions(+), 10 deletions(-) diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index fa4f101c522..24b8ff87862 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -1620,11 +1620,15 @@ class EditableTextState extends State with AutomaticKeepAliveClien @override TextEditingValue get currentTextEditingValue => _value; + bool _updateEditingValueInProgress = false; + @override void updateEditingValue(TextEditingValue value) { + _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) { @@ -1643,7 +1647,14 @@ class EditableTextState extends State with AutomaticKeepAliveClien } } - if (_isSelectionOnlyChange(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) { + // `selection` is the only change. _handleSelectionChanged(value.selection, renderEditable!, SelectionChangedCause.keyboard); } else { _formatAndSetValue(value); @@ -1655,10 +1666,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien _stopCursorTimer(resetCharTicks: false); _startCursorTimer(); } - } - - bool _isSelectionOnlyChange(TextEditingValue value) { - return value.text == _value.text && value.composing == _value.composing; + _updateEditingValueInProgress = false; } @override @@ -1815,8 +1823,14 @@ class EditableTextState extends State with AutomaticKeepAliveClien if (!_hasInputConnection) return; final TextEditingValue localValue = _value; - if (localValue == _receivedRemoteTextEditingValue) + // 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); } @@ -2140,10 +2154,6 @@ class EditableTextState extends State with AutomaticKeepAliveClien _value = _lastFormattedValue!; } - // Always attempt to send the value. If the value has changed, then it will send, - // otherwise, it will short-circuit. - _updateRemoteEditingValueIfNeeded(); - if (textChanged && widget.onChanged != null) widget.onChanged!(value.text); _lastFormattedUnmodifiedTextEditingValue = _receivedRemoteTextEditingValue; diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index e638a358363..9f6ed0e3094 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -4713,6 +4713,132 @@ void main() { expect(tester.testTextInput.editingState['text'], 'flutter is the best!...'); }); + testWidgets('Synchronous test of local and remote editing values', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/65059 + final List log = []; + SystemChannels.textInput.setMockMethodCallHandler((MethodCall methodCall) async { + log.add(methodCall); + }); + final TextInputFormatter formatter = TextInputFormatter.withFunction((TextEditingValue oldValue, TextEditingValue newValue) { + if (newValue.text == 'I will be modified by the formatter.') { + newValue = const TextEditingValue(text: 'Flutter is the best!'); + } + return newValue; + }); + final TextEditingController controller = TextEditingController(); + StateSetter setState; + + final FocusNode focusNode = FocusNode(debugLabel: 'EditableText Focus Node'); + Widget builder() { + return StatefulBuilder( + builder: (BuildContext context, StateSetter setter) { + setState = setter; + return MaterialApp( + home: MediaQuery( + data: const MediaQueryData(devicePixelRatio: 1.0), + child: Directionality( + textDirection: TextDirection.ltr, + child: Center( + child: Material( + child: EditableText( + controller: controller, + focusNode: focusNode, + style: textStyle, + cursorColor: Colors.red, + backgroundCursorColor: Colors.red, + keyboardType: TextInputType.multiline, + inputFormatters: [ + formatter, + ], + onChanged: (String value) { }, + ), + ), + ), + ), + ), + ); + }, + ); + } + + await tester.pumpWidget(builder()); + await tester.tap(find.byType(EditableText)); + await tester.showKeyboard(find.byType(EditableText)); + await tester.pump(); + + log.clear(); + + final EditableTextState state = tester.firstState(find.byType(EditableText)); + + // setEditingState is not called when only the remote changes + state.updateEditingValue(const TextEditingValue( + text: 'a', + )); + expect(log.length, 0); + + // setEditingState is called when remote value modified by the formatter. + state.updateEditingValue(const TextEditingValue( + text: 'I will be modified by the formatter.', + )); + expect(log.length, 1); + MethodCall methodCall = log[0]; + expect( + methodCall, + isMethodCall('TextInput.setEditingState', arguments: { + 'text': 'Flutter is the best!', + 'selectionBase': -1, + 'selectionExtent': -1, + 'selectionAffinity': 'TextAffinity.downstream', + 'selectionIsDirectional': false, + 'composingBase': -1, + 'composingExtent': -1, + }), + ); + + log.clear(); + + // setEditingState is called when the [controller.value] is modified by local. + setState(() { + controller.text = 'I love flutter!'; + }); + expect(log.length, 1); + methodCall = log[0]; + expect( + methodCall, + isMethodCall('TextInput.setEditingState', arguments: { + 'text': 'I love flutter!', + 'selectionBase': -1, + 'selectionExtent': -1, + 'selectionAffinity': 'TextAffinity.downstream', + 'selectionIsDirectional': false, + 'composingBase': -1, + 'composingExtent': -1, + }), + ); + + log.clear(); + + // Currently `_receivedRemoteTextEditingValue` equals 'I will be modified by the formatter.', + // setEditingState will be called when set the [controller.value] to `_receivedRemoteTextEditingValue` by local. + setState(() { + controller.text = 'I will be modified by the formatter.'; + }); + expect(log.length, 1); + methodCall = log[0]; + expect( + methodCall, + isMethodCall('TextInput.setEditingState', arguments: { + 'text': 'I will be modified by the formatter.', + 'selectionBase': -1, + 'selectionExtent': -1, + 'selectionAffinity': 'TextAffinity.downstream', + 'selectionIsDirectional': false, + 'composingBase': -1, + 'composingExtent': -1, + }), + ); + }); + testWidgets('autofocus:true on first frame does not throw', (WidgetTester tester) async { final TextEditingController controller = TextEditingController(text: testText); controller.selection = const TextSelection(