Unverified Commit a46139a2 authored by chunhtai's avatar chunhtai Committed by GitHub

fixes TextInputFormatter gets wrong old value of a selection (#75541)

parent b7d48062
......@@ -102,6 +102,7 @@ class _CupertinoTextFieldSelectionGestureDetectorBuilder extends TextSelectionGe
@override
void onSingleTapUp(TapUpDetails details) {
editableText.hideToolbar();
// Because TextSelectionGestureDetector listens to taps that happen on
// widgets in front of it, tapping the clear button will also trigger
// this handler. If the clear button widget recognizes the up event,
......
......@@ -494,6 +494,8 @@ class _SelectableTextState extends State<SelectableText> with AutomaticKeepAlive
});
}
TextSelection? _lastSeenTextSelection;
void _handleSelectionChanged(TextSelection selection, SelectionChangedCause? cause) {
final bool willShowSelectionHandles = _shouldShowSelectionHandles(cause);
if (willShowSelectionHandles != _showSelectionHandles) {
......@@ -501,10 +503,12 @@ class _SelectableTextState extends State<SelectableText> with AutomaticKeepAlive
_showSelectionHandles = willShowSelectionHandles;
});
}
if (widget.onSelectionChanged != null) {
// TODO(chunhtai): The selection may be the same. We should remove this
// check once this is fixed https://github.com/flutter/flutter/issues/76349.
if (widget.onSelectionChanged != null && _lastSeenTextSelection != selection) {
widget.onSelectionChanged!(selection, cause);
}
_lastSeenTextSelection = selection;
switch (Theme.of(context).platform) {
case TargetPlatform.iOS:
......
......@@ -754,12 +754,60 @@ class TextEditingValue {
);
}
/// Indicates what triggered the change in selected text (including changes to
/// the cursor location).
enum SelectionChangedCause {
/// The user tapped on the text and that caused the selection (or the location
/// of the cursor) to change.
tap,
/// The user tapped twice in quick succession on the text and that caused
/// the selection (or the location of the cursor) to change.
doubleTap,
/// The user long-pressed the text and that caused the selection (or the
/// location of the cursor) to change.
longPress,
/// The user force-pressed the text and that caused the selection (or the
/// location of the cursor) to change.
forcePress,
/// The user used the keyboard to change the selection or the location of the
/// cursor.
///
/// Keyboard-triggered selection changes may be caused by the IME as well as
/// by accessibility tools (e.g. TalkBack on Android).
keyboard,
/// The user used the selection toolbar to change the selection or the
/// location of the cursor.
///
/// An example is when the user taps on select all in the tool bar.
toolBar,
/// The user used the mouse to change the selection by dragging over a piece
/// of text.
drag,
}
/// An interface for manipulating the selection, to be used by the implementor
/// of the toolbar widget.
abstract class TextSelectionDelegate {
/// Gets the current text input.
TextEditingValue get textEditingValue;
/// Indicates that the user has requested the delegate to replace its current
/// text editing state with [value].
///
/// The new [value] is treated as user input and thus may subject to input
/// formatting.
@Deprecated(
'Use the userUpdateTextEditingValue instead. '
'This feature was deprecated after v1.26.0-17.2.pre.'
)
set textEditingValue(TextEditingValue value) {}
/// Indicates that the user has requested the delegate to replace its current
/// text editing state with [value].
///
......@@ -768,10 +816,10 @@ abstract class TextSelectionDelegate {
///
/// See also:
///
/// * [EditableTextState.textEditingValue]: an implementation that applies
/// additional pre-processing to the specified [value], before updating the
/// text editing state.
set textEditingValue(TextEditingValue value);
/// * [EditableTextState.userUpdateTextEditingValue]: an implementation that
/// applies additional pre-processing to the specified [value], before
/// updating the text editing state.
void userUpdateTextEditingValue(TextEditingValue value, SelectionChangedCause cause);
/// Hides the text selection toolbar.
void hideToolbar();
......
......@@ -205,12 +205,15 @@ abstract class TextSelectionControls {
Clipboard.setData(ClipboardData(
text: value.selection.textInside(value.text),
));
delegate.textEditingValue = TextEditingValue(
text: value.selection.textBefore(value.text)
+ value.selection.textAfter(value.text),
selection: TextSelection.collapsed(
offset: value.selection.start
delegate.userUpdateTextEditingValue(
TextEditingValue(
text: value.selection.textBefore(value.text)
+ value.selection.textAfter(value.text),
selection: TextSelection.collapsed(
offset: value.selection.start
)
),
SelectionChangedCause.toolBar,
);
delegate.bringIntoView(delegate.textEditingValue.selection.extent);
delegate.hideToolbar();
......@@ -228,9 +231,12 @@ abstract class TextSelectionControls {
text: value.selection.textInside(value.text),
));
clipboardStatus?.update();
delegate.textEditingValue = TextEditingValue(
text: value.text,
selection: TextSelection.collapsed(offset: value.selection.end),
delegate.userUpdateTextEditingValue(
TextEditingValue(
text: value.text,
selection: TextSelection.collapsed(offset: value.selection.end),
),
SelectionChangedCause.toolBar,
);
delegate.bringIntoView(delegate.textEditingValue.selection.extent);
delegate.hideToolbar();
......@@ -251,13 +257,16 @@ abstract class TextSelectionControls {
final TextEditingValue value = delegate.textEditingValue; // Snapshot the input before using `await`.
final ClipboardData? data = await Clipboard.getData(Clipboard.kTextPlain);
if (data != null) {
delegate.textEditingValue = TextEditingValue(
text: value.selection.textBefore(value.text)
+ data.text!
+ value.selection.textAfter(value.text),
selection: TextSelection.collapsed(
offset: value.selection.start + data.text!.length
delegate.userUpdateTextEditingValue(
TextEditingValue(
text: value.selection.textBefore(value.text)
+ data.text!
+ value.selection.textAfter(value.text),
selection: TextSelection.collapsed(
offset: value.selection.start + data.text!.length
),
),
SelectionChangedCause.toolBar,
);
}
delegate.bringIntoView(delegate.textEditingValue.selection.extent);
......@@ -272,12 +281,15 @@ abstract class TextSelectionControls {
/// This is called by subclasses when their select-all affordance is activated
/// by the user.
void handleSelectAll(TextSelectionDelegate delegate) {
delegate.textEditingValue = TextEditingValue(
text: delegate.textEditingValue.text,
selection: TextSelection(
baseOffset: 0,
extentOffset: delegate.textEditingValue.text.length,
delegate.userUpdateTextEditingValue(
TextEditingValue(
text: delegate.textEditingValue.text,
selection: TextSelection(
baseOffset: 0,
extentOffset: delegate.textEditingValue.text.length,
),
),
SelectionChangedCause.toolBar,
);
delegate.bringIntoView(delegate.textEditingValue.selection.extent);
}
......@@ -436,13 +448,16 @@ class TextSelectionOverlay {
/// Builds the handles by inserting them into the [context]'s overlay.
void showHandles() {
assert(_handles == null);
if (_handles != null)
return;
_handles = <OverlayEntry>[
OverlayEntry(builder: (BuildContext context) => _buildHandle(context, _TextSelectionHandlePosition.start)),
OverlayEntry(builder: (BuildContext context) => _buildHandle(context, _TextSelectionHandlePosition.end)),
];
Overlay.of(context, rootOverlay: true, debugRequiredFor: debugRequiredFor)!.insertAll(_handles!);
Overlay.of(context, rootOverlay: true, debugRequiredFor: debugRequiredFor)!
.insertAll(_handles!);
}
/// Destroys the handles by removing them from overlay.
......@@ -613,10 +628,13 @@ class TextSelectionOverlay {
textPosition = newSelection.base;
break;
case _TextSelectionHandlePosition.end:
textPosition =newSelection.extent;
textPosition = newSelection.extent;
break;
}
selectionDelegate!.textEditingValue = _value.copyWith(selection: newSelection, composing: TextRange.empty);
selectionDelegate!.userUpdateTextEditingValue(
_value.copyWith(selection: newSelection, composing: TextRange.empty),
SelectionChangedCause.drag,
);
selectionDelegate!.bringIntoView(textPosition);
}
}
......
......@@ -3475,7 +3475,6 @@ void main() {
from: tester.getTopRight(find.byType(CupertinoApp)),
cause: SelectionChangedCause.tap,
);
expect(state.showToolbar(), true);
await tester.pumpAndSettle();
// -1 because we want to reach the end of the line, not the start of a new line.
......@@ -3536,7 +3535,6 @@ void main() {
from: tester.getCenter(find.byType(EditableText)),
cause: SelectionChangedCause.tap,
);
expect(state.showToolbar(), true);
await tester.pumpAndSettle();
bottomLeftSelectionPosition = textOffsetToBottomLeftPosition(tester, state.renderEditable.selection!.baseOffset);
......
......@@ -119,6 +119,8 @@ void main() {
expect(tester.testTextInput.isVisible, isTrue);
tester.testTextInput.hide();
final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText));
state.connectionClosed();
expect(tester.testTextInput.isVisible, isFalse);
......
......@@ -18,6 +18,8 @@ import '../widgets/editable_text_utils.dart' show findRenderEditable, globalize,
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';
typedef FormatEditUpdateCallback = void Function(TextEditingValue, TextEditingValue);
class MockClipboard {
Object _clipboardData = <String, dynamic>{
'text': null,
......@@ -127,6 +129,16 @@ double getOpacity(WidgetTester tester, Finder finder) {
).opacity.value;
}
class TestFormatter extends TextInputFormatter {
TestFormatter(this.onFormatEditUpdate);
FormatEditUpdateCallback onFormatEditUpdate;
@override
TextEditingValue formatEditUpdate(TextEditingValue oldValue, TextEditingValue newValue) {
onFormatEditUpdate(oldValue, newValue);
return newValue;
}
}
void main() {
TestWidgetsFlutterBinding.ensureInitialized();
final MockClipboard mockClipboard = MockClipboard();
......@@ -474,6 +486,47 @@ void main() {
);
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS }));
testWidgets('TextInputFormatter gets correct selection value', (WidgetTester tester) async {
late TextEditingValue actualOldValue;
late TextEditingValue actualNewValue;
final FormatEditUpdateCallback callBack = (TextEditingValue oldValue, TextEditingValue newValue) {
actualOldValue = oldValue;
actualNewValue = newValue;
};
final FocusNode focusNode = FocusNode();
final TextEditingController controller = TextEditingController(text: '123');
await tester.pumpWidget(
boilerplate(
child: TextField(
controller: controller,
focusNode: focusNode,
inputFormatters: <TextInputFormatter>[TestFormatter(callBack)],
),
),
);
await tester.tap(find.byType(TextField));
await tester.pumpAndSettle();
await tester.sendKeyEvent(LogicalKeyboardKey.backspace);
await tester.pumpAndSettle();
expect(
actualOldValue,
const TextEditingValue(
text: '123',
selection: TextSelection.collapsed(offset: 3, affinity: TextAffinity.upstream),
),
);
expect(
actualNewValue,
const TextEditingValue(
text: '12',
selection: TextSelection.collapsed(offset: 2),
),
);
});
testWidgets('text field selection toolbar renders correctly inside opacity', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
......@@ -1071,11 +1124,9 @@ void main() {
));
expect(find.text('Paste'), findsNothing);
final Offset emptyPos = textOffsetToPosition(tester, 0);
await tester.longPressAt(emptyPos, pointer: 7);
await tester.pumpAndSettle();
expect(find.text('Paste'), findsOneWidget);
});
......
......@@ -18,6 +18,9 @@ class FakeEditableTextState with TextSelectionDelegate {
@override
TextEditingValue textEditingValue = TextEditingValue.empty;
@override
void userUpdateTextEditingValue(TextEditingValue value, SelectionChangedCause cause) { }
@override
void hideToolbar() { }
......
......@@ -48,6 +48,7 @@ void main() {
});
testWidgets('cursor layout has correct width', (WidgetTester tester) async {
EditableText.debugDeterministicCursor = true;
final GlobalKey<EditableTextState> editableTextKey = GlobalKey<EditableTextState>();
late String changedValue;
......@@ -87,8 +88,7 @@ void main() {
await tester.pumpAndSettle();
await tester.tap(find.text('Paste'));
// Wait for cursor to appear.
await tester.pump(const Duration(milliseconds: 600));
await tester.pump();
expect(changedValue, clipboardContent);
......@@ -96,6 +96,7 @@ void main() {
find.byKey(const ValueKey<int>(1)),
matchesGoldenFile('editable_text_test.0.png'),
);
EditableText.debugDeterministicCursor = false;
});
testWidgets('cursor layout has correct radius', (WidgetTester tester) async {
......@@ -787,6 +788,7 @@ void main() {
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS }));
testWidgets('cursor layout', (WidgetTester tester) async {
EditableText.debugDeterministicCursor = true;
final GlobalKey<EditableTextState> editableTextKey = GlobalKey<EditableTextState>();
late String changedValue;
......@@ -831,8 +833,7 @@ void main() {
await tester.pumpAndSettle();
await tester.tap(find.text('Paste'));
// Wait for cursor to appear.
await tester.pump(const Duration(milliseconds: 600));
await tester.pump();
expect(changedValue, clipboardContent);
......@@ -840,9 +841,11 @@ void main() {
find.byKey(const ValueKey<int>(1)),
matchesGoldenFile('editable_text_test.2.png'),
);
EditableText.debugDeterministicCursor = false;
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS }));
testWidgets('cursor layout has correct height', (WidgetTester tester) async {
EditableText.debugDeterministicCursor = true;
final GlobalKey<EditableTextState> editableTextKey = GlobalKey<EditableTextState>();
late String changedValue;
......@@ -888,8 +891,7 @@ void main() {
await tester.pumpAndSettle();
await tester.tap(find.text('Paste'));
// Wait for cursor to appear.
await tester.pump(const Duration(milliseconds: 600));
await tester.pump();
expect(changedValue, clipboardContent);
......@@ -897,5 +899,6 @@ void main() {
find.byKey(const ValueKey<int>(1)),
matchesGoldenFile('editable_text_test.3.png'),
);
EditableText.debugDeterministicCursor = false;
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS }));
}
......@@ -5199,7 +5199,7 @@ void main() {
tester.testTextInput.log.clear();
final EditableTextState state = tester.state<EditableTextState>(find.byWidget(editableText));
state.textEditingValue = const TextEditingValue(text: 'remoteremoteremote');
state.userUpdateTextEditingValue(const TextEditingValue(text: 'remoteremoteremote'), SelectionChangedCause.keyboard);
// Apply in order: length formatter -> listener -> onChanged -> listener.
expect(controller.text, 'remote listener onChanged listener');
......@@ -5355,6 +5355,7 @@ void main() {
'TextInput.setEditingState',
'TextInput.setEditingState',
'TextInput.show',
'TextInput.show',
];
expect(tester.testTextInput.log.length, logOrder.length);
int index = 0;
......@@ -5469,16 +5470,18 @@ void main() {
log.clear();
final EditableTextState state = tester.firstState(find.byType(EditableText));
// setEditingState is not called when only the remote changes
state.updateEditingValue(const TextEditingValue(
state.updateEditingValue(TextEditingValue(
text: 'a',
selection: controller.selection,
));
expect(log.length, 0);
// setEditingState is called when remote value modified by the formatter.
state.updateEditingValue(const TextEditingValue(
state.updateEditingValue(TextEditingValue(
text: 'I will be modified by the formatter.',
selection: controller.selection,
));
expect(log.length, 1);
MethodCall methodCall = log[0];
......@@ -5592,8 +5595,9 @@ void main() {
final EditableTextState state = tester.firstState(find.byType(EditableText));
// setEditingState is called when remote value modified by the formatter.
state.updateEditingValue(const TextEditingValue(
state.updateEditingValue(TextEditingValue(
text: 'I will be modified by the formatter.',
selection: controller.selection,
));
expect(log.length, 1);
expect(log, contains(matchesMethodCall(
......@@ -5665,8 +5669,9 @@ void main() {
final EditableTextState state = tester.firstState(find.byType(EditableText));
state.updateEditingValue(const TextEditingValue(
state.updateEditingValue(TextEditingValue(
text: 'a',
selection: controller.selection,
));
await tester.pump();
......@@ -5689,8 +5694,9 @@ void main() {
log.clear();
// Send repeat value from the engine.
state.updateEditingValue(const TextEditingValue(
state.updateEditingValue(TextEditingValue(
text: 'a',
selection: controller.selection,
));
await tester.pump();
......@@ -5784,8 +5790,9 @@ void main() {
final EditableTextState state = tester.firstState(find.byType(EditableText));
state.updateEditingValue(const TextEditingValue(
state.updateEditingValue(TextEditingValue(
text: 'a',
selection: controller.selection,
));
await tester.pump();
......@@ -6579,6 +6586,7 @@ void main() {
final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText));
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
selection: TextSelection.collapsed(offset: 4),
composing: TextRange(start: 4, end: 12),
));
controller.selection = const TextSelection.collapsed(offset: 2);
......@@ -6587,6 +6595,7 @@ void main() {
// Reset the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
selection: TextSelection.collapsed(offset: 4),
composing: TextRange(start: 4, end: 12),
));
expect(state.currentTextEditingValue.composing, const TextRange(start: 4, end: 12));
......@@ -6594,13 +6603,14 @@ void main() {
// Positioning cursor after the composing range should clear the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
selection: TextSelection.collapsed(offset: 4),
composing: TextRange(start: 4, end: 12),
));
controller.selection = const TextSelection.collapsed(offset: 14);
expect(state.currentTextEditingValue.composing, TextRange.empty);
});
testWidgets('Clears composing range if cursor moves outside that range', (WidgetTester tester) async {
testWidgets('Clears composing range if cursor moves outside that range - case two', (WidgetTester tester) async {
final Widget widget = MaterialApp(
home: EditableText(
backgroundCursorColor: Colors.grey,
......@@ -6617,6 +6627,7 @@ void main() {
final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText));
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
selection: TextSelection.collapsed(offset: 4),
composing: TextRange(start: 4, end: 12),
));
controller.selection = const TextSelection(baseOffset: 1, extentOffset: 2);
......@@ -6625,6 +6636,7 @@ void main() {
// Reset the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
selection: TextSelection.collapsed(offset: 4),
composing: TextRange(start: 4, end: 12),
));
expect(state.currentTextEditingValue.composing, const TextRange(start: 4, end: 12));
......@@ -6632,6 +6644,7 @@ void main() {
// Setting a selection within the composing range clears the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
selection: TextSelection.collapsed(offset: 4),
composing: TextRange(start: 4, end: 12),
));
controller.selection = const TextSelection(baseOffset: 5, extentOffset: 7);
......@@ -6640,6 +6653,7 @@ void main() {
// Reset the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
selection: TextSelection.collapsed(offset: 4),
composing: TextRange(start: 4, end: 12),
));
expect(state.currentTextEditingValue.composing, const TextRange(start: 4, end: 12));
......@@ -6647,6 +6661,7 @@ void main() {
// Setting a selection after the composing range clears the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
selection: TextSelection.collapsed(offset: 4),
composing: TextRange(start: 4, end: 12),
));
controller.selection = const TextSelection(baseOffset: 13, extentOffset: 15);
......
......@@ -797,6 +797,7 @@ class FakeRenderEditable extends RenderEditable {
),
startHandleLayerLink: LayerLink(),
endHandleLayerLink: LayerLink(),
ignorePointer: true,
textAlign: TextAlign.start,
textDirection: TextDirection.ltr,
locale: const Locale('en', 'US'),
......
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