Improved behaviour for text-editing widgets (#12273)

This patch fixes a collection of issues with widgets involved in text
editing:

  * Fire widget.onChanged on EditableText value change:
    The value of an EditableText is composed of the text value as well
    as other editing-related data such as selection-related information.

    Previously, widget.onChanged() was only called for updates via
    updateEditingValue(). For pastes via a TextSelectionOverlay, updates
    are signalled via _handleSelectionOverlayChanged(), which only ever
    triggered widget.onSelectionChanged(), but not widget.onChanged().

    Both updateEditingValue() and _handleSelectionOverlayChanged()
    perform the value update via _formatAndSetValue(), which is where
    this patch moves the widget.onChanged() call.

  * Correctly update TextFormField value on edits via controller:
    The textual value of a TextFormField exists in two locations:
      1. FormField.value, as with all FormFields and subclasses.
      2. TextEditingController.value associated with the TextField
         underlying the TextFormField.

    Previously, edits to the TextEditingController associated with a
    TextFormField resulted in updates to the rendered TextField widget,
    but did not update TextFormField.value. FormField.value is updated
    via FormField's onChanged function, which is called from the
    EditableText underlying the TextField underlying the TextFormField.
    EditableText only fires onChanged when it receives changes from the
    engine. It does not fire onChanged for changes made to the
    underlying TextController, since the owner of the TextController is
    the one making these changes and thus, already aware of them.
    FormField, however, *does* need to listen to these changes to update
    its value.

  * Adds an initialValue parameter to the TextFormField constructor:
    FormField's constructor already takes an initialValue parameter,
    which specifies the initial value in the field, which is also the
    value to which reset() returns the field.

    Previously, TextFormField took its initial value from the controller
    value (if a controller was passed in) or the empty string (if not).
    This had the undesirable effect that calling reset() always resets
    the value to the current value of the controller... i.e., does
    nothing.

    We now take an initial value explicitly.

packages/flutter/lib/src/material/text_form_field.dart

@@ -10,7 +10,7 @@ import 'text_field.dart';
 
 /// A [FormField] that contains a [TextField].
 ///
-/// This is a convenience widget that simply wraps a [TextField] widget in a
+/// This is a convenience widget that wraps a [TextField] widget in a
 /// [FormField].
 ///
 /// A [Form] ancestor is not required. The [Form] simply makes it easier to
@@ -18,6 +18,10 @@ import 'text_field.dart';
 /// pass a [GlobalKey] to the constructor and use [GlobalKey.currentState] to
 /// save or reset the form field.
 ///
+/// When a [controller] is specified, it can be used to control the text being
+/// edited. Its content will be overwritten by [initialValue] (which defaults
+/// to the empty string) on creation and when [reset] is called.
+///
 /// For a documentation about the various parameters, see [TextField].
 ///
 /// See also:
@@ -30,11 +34,16 @@ import 'text_field.dart';
 class TextFormField extends FormField<String> {
   /// Creates a [FormField] that contains a [TextField].
   ///
+  /// When a [controller] is specified, it can be used to control the text
+  /// being edited. Its content will be overwritten by [initialValue] (which
+  /// defaults to the empty string) on creation and when [reset] is called.
+  ///
   /// For documentation about the various parameters, see the [TextField] class
   /// and [new TextField], the constructor.
   TextFormField({
     Key key,
-    TextEditingController controller,
+    this.controller,
+    String initialValue: '',
     FocusNode focusNode,
     InputDecoration decoration: const InputDecoration(),
     TextInputType keyboardType: TextInputType.text,
@@ -46,19 +55,21 @@ class TextFormField extends FormField<String> {
     FormFieldSetter<String> onSaved,
     FormFieldValidator<String> validator,
     List<TextInputFormatter> inputFormatters,
-  }) : assert(keyboardType != null),
+  }) : assert(initialValue != null),
+       assert(keyboardType != null),
        assert(autofocus != null),
        assert(obscureText != null),
        assert(autocorrect != null),
        assert(maxLines == null || maxLines > 0),
        super(
     key: key,
-    initialValue: controller != null ? controller.value.text : '',
+    initialValue: initialValue,
     onSaved: onSaved,
     validator: validator,
     builder: (FormFieldState<String> field) {
+      final _TextFormFieldState state = field;
       return new TextField(
-        controller: controller,
+        controller: state._effectiveController,
         focusNode: focusNode,
         decoration: decoration.copyWith(errorText: field.errorText),
         keyboardType: keyboardType,
@@ -72,4 +83,75 @@ class TextFormField extends FormField<String> {
       );
     },
   );
+
+  /// Controls the text being edited.
+  ///
+  /// If null, this widget will create its own [TextEditingController].
+  final TextEditingController controller;
+
+  @override
+  _TextFormFieldState createState() => new _TextFormFieldState();
+}
+
+class _TextFormFieldState extends FormFieldState<String> {
+  TextEditingController _controller;
+
+  TextEditingController get _effectiveController => widget.controller ?? _controller;
+
+  @override
+  TextFormField get widget => super.widget;
+
+  @override
+  void initState() {
+    super.initState();
+    if (widget.controller == null) {
+      _controller = new TextEditingController(text: widget.initialValue);
+    } else {
+      widget.controller.text = widget.initialValue;
+      widget.controller.addListener(_handleControllerChanged);
+    }
+  }
+
+  @override
+  void didUpdateWidget(TextFormField oldWidget) {
+    super.didUpdateWidget(oldWidget);
+    if (widget.controller != oldWidget.controller) {
+      oldWidget.controller?.removeListener(_handleControllerChanged);
+      widget.controller?.addListener(_handleControllerChanged);
+
+      if (oldWidget.controller != null && widget.controller == null)
+        _controller = new TextEditingController.fromValue(oldWidget.controller.value);
+      if (widget.controller != null) {
+        setValue(widget.controller.text);
+        if (oldWidget.controller == null)
+          _controller = null;
+      }
+    }
+  }
+
+  @override
+  void dispose() {
+    widget.controller?.removeListener(_handleControllerChanged);
+    super.dispose();
+  }
+
+  @override
+  void reset() {
+    super.reset();
+    setState(() {
+      _effectiveController.text = widget.initialValue;
+    });
+  }
+
+  void _handleControllerChanged() {
+    // Suppress changes that originated from within this class.
+    //
+    // In the case where a controller has been passed in to this widget, we
+    // register this change listener. In these cases, we'll also receive change
+    // notifications for changes originating from within this class -- for
+    // example, the reset() method. In such cases, the FormField value will
+    // already have been set.
+    if (_effectiveController.text != value)
+      onChanged(_effectiveController.text);
+  }
 }

packages/flutter/lib/src/widgets/editable_text.dart

@@ -375,8 +375,6 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
     }
     _lastKnownRemoteTextEditingValue = value;
     _formatAndSetValue(value);
-    if (widget.onChanged != null)
-      widget.onChanged(value.text);
   }
 
   @override
@@ -544,6 +542,7 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
   }
 
   void _formatAndSetValue(TextEditingValue value) {
+    final bool textChanged = _value?.text != value?.text;
     if (widget.inputFormatters != null && widget.inputFormatters.isNotEmpty) {
       for (TextInputFormatter formatter in widget.inputFormatters)
         value = formatter.formatEditUpdate(_value, value);
@@ -552,6 +551,8 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
     } else {
       _value = value;
     }
+    if (textChanged && widget.onChanged != null)
+      widget.onChanged(value.text);
   }
 
   /// Whether the blinking cursor is actually visible at this precise moment

packages/flutter/lib/src/widgets/form.dart

@@ -289,6 +289,18 @@ class FormFieldState<T> extends State<FormField<T>> {
     Form.of(context)?._fieldDidChange();
   }
 
+  /// Sets the value associated with this form field.
+  ///
+  /// This method should be only be called by subclasses that need to update
+  /// the form field value due to state changes identified during the widget
+  /// build phase, when calling `setState` is prohibited. In all other cases,
+  /// the value should be set by a call to [onChanged], which ensures that
+  /// `setState` is called.
+  @protected
+  void setValue(T value) {
+    _value = value;
+  }
+
   @override
   void initState() {
     super.initState();

packages/flutter/test/widgets/editable_text_test.dart

@@ -155,6 +155,45 @@ void main() {
     expect(tester.testTextInput.setClientArgs['inputAction'], equals('TextInputAction.done'));
   });
 
+  testWidgets('Fires onChanged when text changes via TextSelectionOverlay', (WidgetTester tester) async {
+    final GlobalKey<EditableTextState> editableTextKey = new GlobalKey<EditableTextState>();
+
+    String changedValue;
+    final Widget widget = new MaterialApp(
+      home: new EditableText(
+        key: editableTextKey,
+        controller: new TextEditingController(),
+        focusNode: new FocusNode(),
+        style: new Typography(platform: TargetPlatform.android).black.subhead,
+        cursorColor: Colors.blue,
+        selectionControls: materialTextSelectionControls,
+        keyboardType: TextInputType.text,
+        onChanged: (String value) {
+          changedValue = value;
+        },
+      ),
+    );
+    await tester.pumpWidget(widget);
+
+    // Populate a fake clipboard.
+    const String clipboardContent = 'Dobunezumi mitai ni utsukushiku naritai';
+    SystemChannels.platform.setMockMethodCallHandler((MethodCall methodCall) async {
+      if (methodCall.method == 'Clipboard.getData')
+        return const <String, dynamic>{ 'text': clipboardContent };
+      return null;
+    });
+
+    // Long-press to bring up the text editing controls.
+    final Finder textFinder = find.byKey(editableTextKey);
+    await tester.longPress(textFinder);
+    await tester.pump();
+
+    await tester.tap(find.text('PASTE'));
+    await tester.pump();
+
+    expect(changedValue, clipboardContent);
+  });
+
   testWidgets('Changing controller updates EditableText', (WidgetTester tester) async {
     final GlobalKey<EditableTextState> editableTextKey = new GlobalKey<EditableTextState>();
     final TextEditingController controller1 = new TextEditingController(text: 'Wibble');
@@ -211,4 +250,43 @@ void main() {
       }),
     ]);
   });
+
+  testWidgets('Fires onChanged when text changes via TextSelectionOverlay', (WidgetTester tester) async {
+    final GlobalKey<EditableTextState> editableTextKey = new GlobalKey<EditableTextState>();
+
+    String changedValue;
+    final Widget widget = new MaterialApp(
+      home: new EditableText(
+        key: editableTextKey,
+        controller: new TextEditingController(),
+        focusNode: new FocusNode(),
+        style: new Typography(platform: TargetPlatform.android).black.subhead,
+        cursorColor: Colors.blue,
+        selectionControls: materialTextSelectionControls,
+        keyboardType: TextInputType.text,
+        onChanged: (String value) {
+          changedValue = value;
+        },
+      ),
+    );
+    await tester.pumpWidget(widget);
+
+    // Populate a fake clipboard.
+    const String clipboardContent = 'Dobunezumi mitai ni utsukushiku naritai';
+    SystemChannels.platform.setMockMethodCallHandler((MethodCall methodCall) async {
+      if (methodCall.method == 'Clipboard.getData')
+        return const <String, dynamic>{ 'text': clipboardContent };
+      return null;
+    });
+
+    // Long-press to bring up the text editing controls.
+    final Finder textFinder = find.byKey(editableTextKey);
+    await tester.longPress(textFinder);
+    await tester.pump();
+
+    await tester.tap(find.text('PASTE'));
+    await tester.pump();
+
+    expect(changedValue, clipboardContent);
+  });
 }

packages/flutter/test/widgets/form_test.dart

@@ -165,9 +165,8 @@ void main() {
     await checkErrorText('');
   });
 
-  testWidgets('Provide initial value to input', (WidgetTester tester) async {
+  testWidgets('Provide initial value to input when no controller is specified', (WidgetTester tester) async {
     final String initialValue = 'hello';
-    final TextEditingController controller = new TextEditingController(text: initialValue);
     final GlobalKey<FormFieldState<String>> inputKey = new GlobalKey<FormFieldState<String>>();
 
     Widget builder() {
@@ -178,6 +177,47 @@ void main() {
             child: new Form(
               child: new TextFormField(
                 key: inputKey,
+                initialValue: 'hello',
+              ),
+            ),
+          ),
+        ),
+      );
+    }
+
+    await tester.pumpWidget(builder());
+    await tester.showKeyboard(find.byType(TextFormField));
+
+    // initial value should be loaded into keyboard editing state
+    expect(tester.testTextInput.editingState, isNotNull);
+    expect(tester.testTextInput.editingState['text'], equals(initialValue));
+
+    // initial value should also be visible in the raw input line
+    final EditableTextState editableText = tester.state(find.byType(EditableText));
+    expect(editableText.widget.controller.text, equals(initialValue));
+
+    // sanity check, make sure we can still edit the text and everything updates
+    expect(inputKey.currentState.value, equals(initialValue));
+    await tester.enterText(find.byType(TextFormField), 'world');
+    await tester.pump();
+    expect(inputKey.currentState.value, equals('world'));
+    expect(editableText.widget.controller.text, equals('world'));
+  });
+
+  testWidgets('Provide initial value to input when controller is specified', (WidgetTester tester) async {
+    final TextEditingController controller = new TextEditingController();
+    final String initialValue = 'hello';
+    final GlobalKey<FormFieldState<String>> inputKey = new GlobalKey<FormFieldState<String>>();
+
+    Widget builder() {
+      return new Directionality(
+        textDirection: TextDirection.ltr,
+        child: new Center(
+          child: new Material(
+            child: new Form(
+              child: new TextFormField(
+                key: inputKey,
+                initialValue: 'hello',
                 controller: controller,
               ),
             ),
@@ -196,6 +236,7 @@ void main() {
     // initial value should also be visible in the raw input line
     final EditableTextState editableText = tester.state(find.byType(EditableText));
     expect(editableText.widget.controller.text, equals(initialValue));
+    expect(controller.text, equals(initialValue));
 
     // sanity check, make sure we can still edit the text and everything updates
     expect(inputKey.currentState.value, equals(initialValue));
@@ -203,6 +244,151 @@ void main() {
     await tester.pump();
     expect(inputKey.currentState.value, equals('world'));
     expect(editableText.widget.controller.text, equals('world'));
+    expect(controller.text, equals('world'));
+  });
+
+  testWidgets('TextFormField resets to its initial value', (WidgetTester tester) async {
+    final GlobalKey<FormState> formKey = new GlobalKey<FormState>();
+    final GlobalKey<FormFieldState<String>> inputKey = new GlobalKey<FormFieldState<String>>();
+    final TextEditingController controller = new TextEditingController(text: 'Plover');
+    const String initialValue = 'Plugh';
+
+    Widget builder() {
+      return new Directionality(
+        textDirection: TextDirection.ltr,
+        child: new Center(
+          child: new Material(
+            child: new Form(
+              key: formKey,
+              child: new TextFormField(
+                key: inputKey,
+                controller: controller,
+                initialValue: initialValue,
+              ),
+            ),
+          ),
+        ),
+      );
+    }
+    await tester.pumpWidget(builder());
+    await tester.showKeyboard(find.byType(TextFormField));
+    final EditableTextState editableText = tester.state(find.byType(EditableText));
+
+    // overwrite initial value.
+    controller.text = 'Xyzzy';
+    await tester.idle();
+    expect(editableText.widget.controller.text, equals('Xyzzy'));
+    expect(inputKey.currentState.value, equals('Xyzzy'));
+    expect(controller.text, equals('Xyzzy'));
+
+    // verify value resets to initialValue on reset.
+    formKey.currentState.reset();
+    await tester.idle();
+    expect(inputKey.currentState.value, equals(initialValue));
+    expect(editableText.widget.controller.text, equals(initialValue));
+    expect(controller.text, equals(initialValue));
+  });
+
+  testWidgets('TextEditingController updates to/from form field value', (WidgetTester tester) async {
+    final TextEditingController controller1 = new TextEditingController(text: 'Foo');
+    final TextEditingController controller2 = new TextEditingController(text: 'Bar');
+    final GlobalKey<FormFieldState<String>> inputKey = new GlobalKey<FormFieldState<String>>();
+
+    TextEditingController currentController;
+    StateSetter setState;
+
+    Widget builder() {
+      return new StatefulBuilder(
+        builder: (BuildContext context, StateSetter setter) {
+          setState = setter;
+          return new Directionality(
+            textDirection: TextDirection.ltr,
+            child: new Center(
+              child: new Material(
+                child: new Form(
+                  child: new TextFormField(
+                    key: inputKey,
+                    controller: currentController,
+                  ),
+                ),
+              ),
+            ),
+          );
+        },
+      );
+    }
+
+    await tester.pumpWidget(builder());
+    await tester.showKeyboard(find.byType(TextFormField));
+
+    // verify initially empty.
+    expect(tester.testTextInput.editingState, isNotNull);
+    expect(tester.testTextInput.editingState['text'], isEmpty);
+    final EditableTextState editableText = tester.state(find.byType(EditableText));
+    expect(editableText.widget.controller.text, isEmpty);
+
+    // verify changing the controller from null to controller1 sets the value.
+    setState(() {
+      currentController = controller1;
+    });
+    await tester.pump();
+    expect(editableText.widget.controller.text, equals('Foo'));
+    expect(inputKey.currentState.value, equals('Foo'));
+
+    // verify changes to controller1 text are visible in text field and set in form value.
+    controller1.text = 'Wobble';
+    await tester.idle();
+    expect(editableText.widget.controller.text, equals('Wobble'));
+    expect(inputKey.currentState.value, equals('Wobble'));
+
+    // verify changes to the field text update the form value and controller1.
+    await tester.enterText(find.byType(TextFormField), 'Wibble');
+    await tester.pump();
+    expect(inputKey.currentState.value, equals('Wibble'));
+    expect(editableText.widget.controller.text, equals('Wibble'));
+    expect(controller1.text, equals('Wibble'));
+
+    // verify that switching from controller1 to controller2 is handled.
+    setState(() {
+      currentController = controller2;
+    });
+    await tester.pump();
+    expect(inputKey.currentState.value, equals('Bar'));
+    expect(editableText.widget.controller.text, equals('Bar'));
+    expect(controller2.text, equals('Bar'));
+    expect(controller1.text, equals('Wibble'));
+
+    // verify changes to controller2 text are visible in text field and set in form value.
+    controller2.text = 'Xyzzy';
+    await tester.idle();
+    expect(editableText.widget.controller.text, equals('Xyzzy'));
+    expect(inputKey.currentState.value, equals('Xyzzy'));
+    expect(controller1.text, equals('Wibble'));
+
+    // verify changes to controller1 text are not visible in text field or set in form value.
+    controller1.text = 'Plugh';
+    await tester.idle();
+    expect(editableText.widget.controller.text, equals('Xyzzy'));
+    expect(inputKey.currentState.value, equals('Xyzzy'));
+    expect(controller1.text, equals('Plugh'));
+
+    // verify that switching from controller2 to null is handled.
+    setState(() {
+      currentController = null;
+    });
+    await tester.pump();
+    expect(inputKey.currentState.value, equals('Xyzzy'));
+    expect(editableText.widget.controller.text, equals('Xyzzy'));
+    expect(controller2.text, equals('Xyzzy'));
+    expect(controller1.text, equals('Plugh'));
+
+    // verify that changes to the field text update the form value but not the previous controllers.
+    await tester.enterText(find.byType(TextFormField), 'Plover');
+    await tester.pump();
+    expect(inputKey.currentState.value, equals('Plover'));
+    expect(editableText.widget.controller.text, equals('Plover'));
+    expect(controller1.text, equals('Plugh'));
+    expect(controller2.text, equals('Xyzzy'));
   });
 
   testWidgets('No crash when a TextFormField is removed from the tree', (WidgetTester tester) async {