Unverified Commit bb02f40c authored by Gary Qian's avatar Gary Qian Committed by GitHub

Remove strict repeat check from framework formatter (moved to engine) (#53974)

parent c7b10df7
...@@ -1679,13 +1679,14 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien ...@@ -1679,13 +1679,14 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
_whitespaceFormatter ??= _WhitespaceDirectionalityFormatter(textDirection: _textDirection); _whitespaceFormatter ??= _WhitespaceDirectionalityFormatter(textDirection: _textDirection);
// Check if the new value is the same as the current local value, or is the same // 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 textChanged = _value?.text != value?.text;
final bool isRepeatText = value?.text == _lastFormattedUnmodifiedTextEditingValue?.text; final bool isRepeat = value == _lastFormattedUnmodifiedTextEditingValue;
final bool isRepeatSelection = value?.selection == _lastFormattedUnmodifiedTextEditingValue?.selection;
final bool isRepeatComposing = value?.composing == _lastFormattedUnmodifiedTextEditingValue?.composing; if (textChanged && widget.inputFormatters != null && widget.inputFormatters.isNotEmpty) {
// Only format when the text has changed and there are available formatters. // Only format when the text has changed and there are available formatters.
if (!isRepeatText && textChanged && widget.inputFormatters != null && widget.inputFormatters.isNotEmpty) { // 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) { for (final TextInputFormatter formatter in widget.inputFormatters) {
value = formatter.formatEditUpdate(_value, value); value = formatter.formatEditUpdate(_value, value);
} }
...@@ -1695,9 +1696,10 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien ...@@ -1695,9 +1696,10 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
_lastFormattedValue = value; _lastFormattedValue = value;
} }
// Setting _value here ensures the selection and composing region info is passed.
_value = value; _value = value;
// Use the last formatted value when an identical repeat pass is detected. // 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; _value = _lastFormattedValue;
} }
......
...@@ -4299,22 +4299,24 @@ void main() { ...@@ -4299,22 +4299,24 @@ void main() {
const List<String> referenceLog = <String>[ const List<String> referenceLog = <String>[
'[1]: , a', '[1]: , a',
'[1]: normal aa', '[1]: normal aa',
'[2]: aa, aaa', '[2]: a, aa',
'[2]: normal aaaa', '[2]: normal aaaa',
'[3]: aaaa, aa', '[3]: aa, aaa',
'[3]: deleting a', '[3]: normal aaaaaa',
'[4]: a, aaa', '[4]: aaa, aa',
'[4]: normal aaaaaaaa', '[4]: deleting aa',
'[5]: aaaaaaaa, aaaa', '[5]: aa, aaa',
'[5]: deleting aaa', '[5]: normal aaaaaaaaaa',
'[6]: aaa, aa', '[6]: aaa, aaaa',
'[6]: deleting aaaa', '[6]: normal aaaaaaaaaaaa',
'[7]: aaaa, aaaaaaa', '[7]: aaaa, aa',
'[7]: normal aaaaaaaaaaaaaa', '[7]: deleting aaaaa',
'[8]: aaaaaaaaaaaaaa, aa', '[8]: aa, aaaaaaa',
'[8]: deleting aaaaaa', '[8]: normal aaaaaaaaaaaaaaaa',
'[9]: aaaaaa, aaaaaaaaa', '[9]: aaaaaaa, aa',
'[9]: normal aaaaaaaaaaaaaaaaaa', '[9]: deleting aaaaaaa',
'[10]: aa, aaaaaaaaa',
'[10]: normal aaaaaaaaaaaaaaaaaaaa'
]; ];
expect(formatter.log, referenceLog); expect(formatter.log, referenceLog);
...@@ -4354,6 +4356,10 @@ void main() { ...@@ -4354,6 +4356,10 @@ void main() {
expect(tester.testTextInput.editingState['text'], equals('')); expect(tester.testTextInput.editingState['text'], equals(''));
expect(state.wantKeepAlive, true); 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); expect(formatter.formatCallCount, 0);
state.updateEditingValue(const TextEditingValue(text: '01')); state.updateEditingValue(const TextEditingValue(text: '01'));
expect(formatter.formatCallCount, 1); expect(formatter.formatCallCount, 1);
...@@ -4361,20 +4367,20 @@ void main() { ...@@ -4361,20 +4367,20 @@ void main() {
expect(formatter.formatCallCount, 2); expect(formatter.formatCallCount, 2);
state.updateEditingValue(const TextEditingValue(text: '0123')); // Text change causes reformat state.updateEditingValue(const TextEditingValue(text: '0123')); // Text change causes reformat
expect(formatter.formatCallCount, 3); 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); 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); expect(formatter.formatCallCount, 3);
state.updateEditingValue(const TextEditingValue(text: '0123', selection: TextSelection.collapsed(offset: 2))); // Selection change does not reformat state.updateEditingValue(const TextEditingValue(text: '0123', selection: TextSelection.collapsed(offset: 2))); // Selection change does not reformat
expect(formatter.formatCallCount, 3); 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); 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); 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 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.formatCallCount, 3);
expect(formatter.lastOldValue.composing, const TextRange(start: -1, end: -1)); 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. state.updateEditingValue(const TextEditingValue(text: '01234', selection: TextSelection.collapsed(offset: 2))); // Formats, with oldValue containing composing region.
expect(formatter.formatCallCount, 4); expect(formatter.formatCallCount, 4);
expect(formatter.lastOldValue.composing, const TextRange(start: 1, end: 2)); expect(formatter.lastOldValue.composing, const TextRange(start: 1, end: 2));
...@@ -4383,9 +4389,9 @@ void main() { ...@@ -4383,9 +4389,9 @@ void main() {
const List<String> referenceLog = <String>[ const List<String> referenceLog = <String>[
'[1]: , 01', '[1]: , 01',
'[1]: normal aa', '[1]: normal aa',
'[2]: aa, 012', '[2]: 01, 012',
'[2]: normal aaaa', '[2]: normal aaaa',
'[3]: aaaa, 0123', '[3]: 012, 0123',
'[3]: normal aaaaaa', '[3]: normal aaaaaa',
'[4]: 0123, 01234', '[4]: 0123, 01234',
'[4]: normal aaaaaaaa' '[4]: normal aaaaaaaa'
...@@ -4694,13 +4700,13 @@ class MockTextFormatter extends TextInputFormatter { ...@@ -4694,13 +4700,13 @@ class MockTextFormatter extends TextInputFormatter {
TextEditingValue oldValue, TextEditingValue newValue) { TextEditingValue oldValue, TextEditingValue newValue) {
final String result = 'a' * (formatCallCount - 2); final String result = 'a' * (formatCallCount - 2);
log.add('[$formatCallCount]: deleting $result'); log.add('[$formatCallCount]: deleting $result');
return TextEditingValue(text: result); return TextEditingValue(text: newValue.text, selection: newValue.selection, composing: newValue.composing);
} }
TextEditingValue _formatText(TextEditingValue value) { TextEditingValue _formatText(TextEditingValue value) {
final String result = 'a' * formatCallCount * 2; final String result = 'a' * formatCallCount * 2;
log.add('[$formatCallCount]: normal $result'); log.add('[$formatCallCount]: normal $result');
return TextEditingValue(text: result); return TextEditingValue(text: value.text, selection: value.selection, composing: value.composing);
} }
} }
......
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