firka/flutter
firka/flutter
4
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert "Don't call onChanged callbacks when formatter rejects the change & handle text input formatter exceptions. (#78707)" (#79590)

Browse Source
This commit is contained in:
LongCatIsLooong 2021-04-01 16:49:02 -07:00 committed by GitHub
parent 8c43ab7ced
commit 11276d060f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 70 additions and 230 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
packages/flutter/lib/src/widgets/editable_text.dart
View File

@ -2236,43 +2236,30 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
_lastBottomViewInset = WidgetsBinding.instance!.window.viewInsets.bottom; _lastBottomViewInset = WidgetsBinding.instance!.window.viewInsets.bottom;
} }
// This method is often called by platform message handlers that catch and void _formatAndSetValue(TextEditingValue value, SelectionChangedCause? cause, {bool userInteraction = false}) {
// send unrecognized exceptions to the engine/platform. Make sure the // Only apply input formatters if the text has changed (including uncommited
// exceptions that user callbacks throw are handled within this method. // text in the composing region), or when the user committed the composing
void _formatAndSetValue(TextEditingValue newTextEditingValue, SelectionChangedCause? cause, {bool userInteraction = false}) { // text.
// Only apply input formatters if the text has changed (including
// uncommitted text in the composing region), or when the user committed
// the composing text.
// Gboard is very persistent in restoring the composing region. Applying // Gboard is very persistent in restoring the composing region. Applying
// input formatters on composing-region-only changes (except clearing the // input formatters on composing-region-only changes (except clearing the
// current composing region) is very infinite-loop-prone: the formatters // current composing region) is very infinite-loop-prone: the formatters
// will keep trying to modify the composing region while Gboard will keep // will keep trying to modify the composing region while Gboard will keep
// trying to restore the original composing region. // trying to restore the original composing region.
final bool preformatTextChanged = _value.text != newTextEditingValue.text final bool textChanged = _value.text != value.text
|| (!_value.composing.isCollapsed && newTextEditingValue.composing.isCollapsed); || (!_value.composing.isCollapsed && value.composing.isCollapsed);
final bool selectionChanged = _value.selection != value.selection;
final List<TextInputFormatter>? formatters = widget.inputFormatters; if (textChanged) {
if (preformatTextChanged && formatters != null && formatters.isNotEmpty) { value = widget.inputFormatters?.fold<TextEditingValue>(
try { value,
for (final TextInputFormatter formatter in formatters) { (TextEditingValue newValue, TextInputFormatter formatter) => formatter.formatEditUpdate(_value, newValue),
newTextEditingValue = formatter.formatEditUpdate(_value, newTextEditingValue); ) ?? value;
}
} catch (exception, stack) {
FlutterError.reportError(FlutterErrorDetails(
exception: exception,
stack: stack,
library: 'widgets',
context: ErrorDescription('while applying TextInputFormatters'),
));
}
} }
// Put all optional user callback invocations in a batch edit to prevent // Put all optional user callback invocations in a batch edit to prevent
// sending multiple `TextInput.updateEditingValue` messages. // sending multiple `TextInput.updateEditingValue` messages.
beginBatchEdit(); beginBatchEdit();
final bool selectionChanged = _value.selection != newTextEditingValue.selection; _value = value;
final bool textChanged = preformatTextChanged && _value != newTextEditingValue;
_value = newTextEditingValue;
// Changes made by the keyboard can sometimes be "out of band" for listening // Changes made by the keyboard can sometimes be "out of band" for listening
// components, so always send those events, even if we didn't think it // components, so always send those events, even if we didn't think it
// changed. Also, the user long pressing should always send a selection change // changed. Also, the user long pressing should always send a selection change
@ -2281,11 +2268,11 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
(userInteraction && (userInteraction &&
(cause == SelectionChangedCause.longPress || (cause == SelectionChangedCause.longPress ||
cause == SelectionChangedCause.keyboard))) { cause == SelectionChangedCause.keyboard))) {
_handleSelectionChanged(newTextEditingValue.selection, cause); _handleSelectionChanged(value.selection, cause);
} }
if (textChanged) { if (textChanged) {
try { try {
widget.onChanged?.call(newTextEditingValue.text); widget.onChanged?.call(value.text);
} catch (exception, stack) { } catch (exception, stack) {
FlutterError.reportError(FlutterErrorDetails( FlutterError.reportError(FlutterErrorDetails(
exception: exception, exception: exception,

257
packages/flutter/test/widgets/editable_text_test.dart
View File

@ -43,7 +43,6 @@ final FocusNode focusNode = FocusNode(debugLabel: 'EditableText Node');
final FocusScopeNode focusScopeNode = FocusScopeNode(debugLabel: 'EditableText Scope Node'); final FocusScopeNode focusScopeNode = FocusScopeNode(debugLabel: 'EditableText Scope Node');
const TextStyle textStyle = TextStyle(); const TextStyle textStyle = TextStyle();
const Color cursorColor = Color.fromARGB(0xFF, 0xFF, 0x00, 0x00); const Color cursorColor = Color.fromARGB(0xFF, 0xFF, 0x00, 0x00);
final TextInputFormatter rejectEverythingFormatter = TextInputFormatter.withFunction((TextEditingValue old, TextEditingValue value) => old);
enum HandlePositionInViewport { enum HandlePositionInViewport {
leftEdge, rightEdge, within, leftEdge, rightEdge, within,
@ -5706,21 +5705,20 @@ void main() {
text: 'I will be modified by the formatter.', text: 'I will be modified by the formatter.',
selection: controller.selection, selection: controller.selection,
)); ));
expect(log, orderedEquals(<dynamic>[ expect(log.length, 1);
matchesMethodCall('TextInput.show'), MethodCall methodCall = log[0];
matchesMethodCall( expect(
'TextInput.setEditingState', methodCall,
args: allOf( isMethodCall('TextInput.setEditingState', arguments: <String, dynamic>{
containsPair('text', 'Flutter is the best!'), 'text': 'Flutter is the best!',
containsPair('selectionBase', -1), 'selectionBase': -1,
containsPair('selectionExtent', -1), 'selectionExtent': -1,
containsPair('selectionAffinity', 'TextAffinity.downstream'), 'selectionAffinity': 'TextAffinity.downstream',
containsPair('selectionIsDirectional', false), 'selectionIsDirectional': false,
containsPair('composingBase', -1), 'composingBase': -1,
containsPair('composingExtent', -1), 'composingExtent': -1,
), }),
), );
]));
log.clear(); log.clear();
@ -5728,21 +5726,21 @@ void main() {
setState(() { setState(() {
controller.text = 'I love flutter!'; controller.text = 'I love flutter!';
}); });
expect(log.length, 1);
methodCall = log[0];
expect(
methodCall,
isMethodCall('TextInput.setEditingState', arguments: <String, dynamic>{
'text': 'I love flutter!',
'selectionBase': -1,
'selectionExtent': -1,
'selectionAffinity': 'TextAffinity.downstream',
'selectionIsDirectional': false,
'composingBase': -1,
'composingExtent': -1,
}),
);
expect(log, equals(<dynamic>[
matchesMethodCall(
'TextInput.setEditingState',
args: allOf(
containsPair('text', 'I love flutter!'),
containsPair('selectionBase', -1),
containsPair('selectionExtent', -1),
containsPair('selectionAffinity', 'TextAffinity.downstream'),
containsPair('selectionIsDirectional', false),
containsPair('composingBase', -1),
containsPair('composingExtent', -1),
),
),
]));
log.clear(); log.clear();
// Currently `_receivedRemoteTextEditingValue` equals 'I will be modified by the formatter.', // Currently `_receivedRemoteTextEditingValue` equals 'I will be modified by the formatter.',
@ -5750,78 +5748,20 @@ void main() {
setState(() { setState(() {
controller.text = 'I will be modified by the formatter.'; controller.text = 'I will be modified by the formatter.';
}); });
expect(log, equals(<dynamic>[ expect(log.length, 1);
matchesMethodCall( methodCall = log[0];
'TextInput.setEditingState', expect(
args: allOf( methodCall,
containsPair('text', 'I will be modified by the formatter.'), isMethodCall('TextInput.setEditingState', arguments: <String, dynamic>{
containsPair('selectionBase', -1), 'text': 'I will be modified by the formatter.',
containsPair('selectionExtent', -1), 'selectionBase': -1,
containsPair('selectionAffinity', 'TextAffinity.downstream'), 'selectionExtent': -1,
containsPair('selectionIsDirectional', false), 'selectionAffinity': 'TextAffinity.downstream',
containsPair('composingBase', -1), 'selectionIsDirectional': false,
containsPair('composingExtent', -1), 'composingBase': -1,
), 'composingExtent': -1,
), }),
])); );
});
testWidgets('Send text input state to engine when the input formatter rejects everything', (WidgetTester tester) async {
final List<MethodCall> log = <MethodCall>[];
SystemChannels.textInput.setMockMethodCallHandler((MethodCall methodCall) async { log.add(methodCall); });
final TextEditingController controller = TextEditingController(text: 'initial text');
final FocusNode focusNode = FocusNode();
Widget builder() {
return StatefulBuilder(
builder: (BuildContext context, StateSetter 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: <TextInputFormatter>[rejectEverythingFormatter],
),
),
),
),
),
);
},
);
}
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));
// The formatter rejects all user inputs, the framework needs to tell the
// engine to restore to the previous editing state.
state.updateEditingValue(TextEditingValue(
text: 'some say kosm',
selection: controller.selection,
));
expect(log, equals(<dynamic>[
matchesMethodCall(
'TextInput.setEditingState',
args: allOf(containsPair('text', 'initial text')),
),
]));
}); });
testWidgets('Send text input state to engine when the input formatter rejects user input', (WidgetTester tester) async { testWidgets('Send text input state to engine when the input formatter rejects user input', (WidgetTester tester) async {
@ -5881,30 +5821,26 @@ void main() {
text: 'I will be modified by the formatter.', text: 'I will be modified by the formatter.',
selection: controller.selection, selection: controller.selection,
)); ));
expect(log, equals(<dynamic>[ expect(log.length, 1);
matchesMethodCall('TextInput.show'), expect(log, contains(matchesMethodCall(
matchesMethodCall( 'TextInput.setEditingState',
'TextInput.setEditingState', args: allOf(
args: allOf( containsPair('text', 'Flutter is the best!'),
containsPair('text', 'Flutter is the best!'),
),
), ),
])); )));
log.clear(); log.clear();
state.updateEditingValue(const TextEditingValue( state.updateEditingValue(const TextEditingValue(
text: 'I will be modified by the formatter.', text: 'I will be modified by the formatter.',
)); ));
expect(log.length, 1);
expect(log, equals(<dynamic>[ expect(log, contains(matchesMethodCall(
matchesMethodCall( 'TextInput.setEditingState',
'TextInput.setEditingState', args: allOf(
args: allOf( containsPair('text', 'Flutter is the best!'),
containsPair('text', 'Flutter is the best!'),
),
), ),
])); )));
}); });
testWidgets('Repeatedly receiving [TextEditingValue] will not trigger a keyboard request', (WidgetTester tester) async { testWidgets('Repeatedly receiving [TextEditingValue] will not trigger a keyboard request', (WidgetTester tester) async {
@ -6972,7 +6908,7 @@ void main() {
}); });
}); });
group('state change user callbacks', () { group('callback errors', () {
const String errorText = 'Test EditableText callback error'; const String errorText = 'Test EditableText callback error';
testWidgets('onSelectionChanged can throw errors', (WidgetTester tester) async { testWidgets('onSelectionChanged can throw errors', (WidgetTester tester) async {
@ -7089,89 +7025,6 @@ void main() {
expect(error, isFlutterError); expect(error, isFlutterError);
expect(error.toString(), contains(errorText)); expect(error.toString(), contains(errorText));
}); });
testWidgets('TextInputFormatters can throw errors', (WidgetTester tester) async {
final TextInputFormatter alwaysThrowFormatter = TextInputFormatter.withFunction(
(TextEditingValue old, TextEditingValue value) {
throw FlutterError(errorText);
},
);
await tester.pumpWidget(MaterialApp(
home: EditableText(
showSelectionHandles: true,
maxLines: 2,
controller: TextEditingController(
text: 'flutter is the best!',
),
focusNode: FocusNode(),
cursorColor: Colors.red,
backgroundCursorColor: Colors.blue,
style: Typography.material2018(platform: TargetPlatform.android).black.subtitle1!.copyWith(fontFamily: 'Roboto'),
keyboardType: TextInputType.text,
inputFormatters: <TextInputFormatter>[alwaysThrowFormatter],
),
));
await tester.enterText(find.byType(EditableText), '...');
final dynamic error = tester.takeException();
expect(error, isFlutterError);
expect(error.toString(), contains(errorText));
});
// Regression test for https://github.com/flutter/flutter/issues/44979.
testWidgets('onChanged callback takes formatter into account', (WidgetTester tester) async {
bool onChangedCalled = false;
await tester.pumpWidget(MaterialApp(
home: EditableText(
showSelectionHandles: true,
maxLines: 2,
controller: TextEditingController(
text: 'flutter is the best!',
),
focusNode: FocusNode(),
cursorColor: Colors.red,
backgroundCursorColor: Colors.blue,
inputFormatters: <TextInputFormatter>[rejectEverythingFormatter],
style: Typography.material2018(platform: TargetPlatform.android).black.subtitle1!.copyWith(fontFamily: 'Roboto'),
keyboardType: TextInputType.text,
onChanged: (String text) {
onChangedCalled = true;
},
),
));
// Modify the text and expect to get rejected.
await tester.enterText(find.byType(EditableText), '...');
expect(onChangedCalled, isFalse);
});
testWidgets('onSelectionChanged callback takes formatter into account', (WidgetTester tester) async {
bool onChangedCalled = false;
await tester.pumpWidget(MaterialApp(
home: EditableText(
showSelectionHandles: true,
maxLines: 2,
controller: TextEditingController(
text: 'flutter is the best!',
),
focusNode: FocusNode(),
cursorColor: Colors.red,
backgroundCursorColor: Colors.blue,
inputFormatters: <TextInputFormatter>[rejectEverythingFormatter],
style: Typography.material2018(platform: TargetPlatform.android).black.subtitle1!.copyWith(fontFamily: 'Roboto'),
keyboardType: TextInputType.text,
onSelectionChanged: (TextSelection selection, SelectionChangedCause? cause) {
onChangedCalled = true;
},
),
));
final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText));
// Modify the text and expect an error from onChanged.
state.updateEditingValue(const TextEditingValue(selection: TextSelection.collapsed(offset: 9)));
expect(onChangedCalled, isFalse);
});
}); });
// Regression test for https://github.com/flutter/flutter/issues/72400. // Regression test for https://github.com/flutter/flutter/issues/72400.