From bb02f40ca8109d42c84a00086f868d4f7e8d068b Mon Sep 17 00:00:00 2001 From: Gary Qian Date: Fri, 10 Apr 2020 13:26:12 -0700 Subject: [PATCH] Remove strict repeat check from framework formatter (moved to engine) (#53974) --- .../lib/src/widgets/editable_text.dart | 16 +++--- .../test/widgets/editable_text_test.dart | 54 ++++++++++--------- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 662115be7c..e1ebab433b 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -1679,13 +1679,14 @@ class EditableTextState extends State with AutomaticKeepAliveClien _whitespaceFormatter ??= _WhitespaceDirectionalityFormatter(textDirection: _textDirection); // Check if the new value is the same as the current local value, or is the same - // as the post-formatting value of the previous pass. + // as the pre-formatting value of the previous pass (repeat call). final bool textChanged = _value?.text != value?.text; - final bool isRepeatText = value?.text == _lastFormattedUnmodifiedTextEditingValue?.text; - final bool isRepeatSelection = value?.selection == _lastFormattedUnmodifiedTextEditingValue?.selection; - final bool isRepeatComposing = value?.composing == _lastFormattedUnmodifiedTextEditingValue?.composing; - // Only format when the text has changed and there are available formatters. - if (!isRepeatText && textChanged && widget.inputFormatters != null && widget.inputFormatters.isNotEmpty) { + final bool isRepeat = value == _lastFormattedUnmodifiedTextEditingValue; + + if (textChanged && 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. for (final TextInputFormatter formatter in widget.inputFormatters) { value = formatter.formatEditUpdate(_value, value); } @@ -1695,9 +1696,10 @@ class EditableTextState extends State with AutomaticKeepAliveClien _lastFormattedValue = value; } + // Setting _value here ensures the selection and composing region info is passed. _value = value; // Use the last formatted value when an identical repeat pass is detected. - if (isRepeatText && isRepeatSelection && isRepeatComposing && textChanged && _lastFormattedValue != null) { + if (isRepeat && textChanged && _lastFormattedValue != null) { _value = _lastFormattedValue; } diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index d13028851c..aeacc0e78a 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -4299,22 +4299,24 @@ void main() { const List referenceLog = [ '[1]: , a', '[1]: normal aa', - '[2]: aa, aaa', + '[2]: a, aa', '[2]: normal aaaa', - '[3]: aaaa, aa', - '[3]: deleting a', - '[4]: a, aaa', - '[4]: normal aaaaaaaa', - '[5]: aaaaaaaa, aaaa', - '[5]: deleting aaa', - '[6]: aaa, aa', - '[6]: deleting aaaa', - '[7]: aaaa, aaaaaaa', - '[7]: normal aaaaaaaaaaaaaa', - '[8]: aaaaaaaaaaaaaa, aa', - '[8]: deleting aaaaaa', - '[9]: aaaaaa, aaaaaaaaa', - '[9]: normal aaaaaaaaaaaaaaaaaa', + '[3]: aa, aaa', + '[3]: normal aaaaaa', + '[4]: aaa, aa', + '[4]: deleting aa', + '[5]: aa, aaa', + '[5]: normal aaaaaaaaaa', + '[6]: aaa, aaaa', + '[6]: normal aaaaaaaaaaaa', + '[7]: aaaa, aa', + '[7]: deleting aaaaa', + '[8]: aa, aaaaaaa', + '[8]: normal aaaaaaaaaaaaaaaa', + '[9]: aaaaaaa, aa', + '[9]: deleting aaaaaaa', + '[10]: aa, aaaaaaaaa', + '[10]: normal aaaaaaaaaaaaaaaaaaaa' ]; expect(formatter.log, referenceLog); @@ -4354,6 +4356,10 @@ void main() { expect(tester.testTextInput.editingState['text'], equals('')); expect(state.wantKeepAlive, true); + // We no longer perform full repeat filtering in framework, it is now left + // to the engine to prevent repeat calls from being sent in the first place. + // Engine preventing repeats is far more reliable and avoids many of the ambiguous + // filtering we performed before. expect(formatter.formatCallCount, 0); state.updateEditingValue(const TextEditingValue(text: '01')); expect(formatter.formatCallCount, 1); @@ -4361,20 +4367,20 @@ void main() { expect(formatter.formatCallCount, 2); state.updateEditingValue(const TextEditingValue(text: '0123')); // Text change causes reformat expect(formatter.formatCallCount, 3); - state.updateEditingValue(const TextEditingValue(text: '0123')); // Repeat, does not format + state.updateEditingValue(const TextEditingValue(text: '0123')); // No text change, does not format expect(formatter.formatCallCount, 3); - state.updateEditingValue(const TextEditingValue(text: '0123')); // Repeat, does not format + state.updateEditingValue(const TextEditingValue(text: '0123')); // No text change, does not format expect(formatter.formatCallCount, 3); state.updateEditingValue(const TextEditingValue(text: '0123', selection: TextSelection.collapsed(offset: 2))); // Selection change does not reformat expect(formatter.formatCallCount, 3); - state.updateEditingValue(const TextEditingValue(text: '0123', selection: TextSelection.collapsed(offset: 2))); // Repeat, does not format + 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))); // Repeat, does not format + 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)); + 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, 4); expect(formatter.lastOldValue.composing, const TextRange(start: 1, end: 2)); @@ -4383,9 +4389,9 @@ void main() { const List referenceLog = [ '[1]: , 01', '[1]: normal aa', - '[2]: aa, 012', + '[2]: 01, 012', '[2]: normal aaaa', - '[3]: aaaa, 0123', + '[3]: 012, 0123', '[3]: normal aaaaaa', '[4]: 0123, 01234', '[4]: normal aaaaaaaa' @@ -4694,13 +4700,13 @@ class MockTextFormatter extends TextInputFormatter { TextEditingValue oldValue, TextEditingValue newValue) { final String result = 'a' * (formatCallCount - 2); log.add('[$formatCallCount]: deleting $result'); - return TextEditingValue(text: result); + return TextEditingValue(text: newValue.text, selection: newValue.selection, composing: newValue.composing); } TextEditingValue _formatText(TextEditingValue value) { final String result = 'a' * formatCallCount * 2; log.add('[$formatCallCount]: normal $result'); - return TextEditingValue(text: result); + return TextEditingValue(text: value.text, selection: value.selection, composing: value.composing); } }