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

Fix rendereditable to check the latest text before setting the selection (#78919)

* Fix rendereditable to check the latest text before setting the selection

* add regression comment

* addressing comments and fix tests
parent 4ccb6f8e
...@@ -564,6 +564,21 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin { ...@@ -564,6 +564,21 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
} }
void _setSelection(TextSelection nextSelection, SelectionChangedCause cause) { void _setSelection(TextSelection nextSelection, SelectionChangedCause cause) {
if (nextSelection.isValid) {
// The nextSelection is calculated based on _plainText, which can be out
// of sync with the textSelectionDelegate.textEditingValue by one frame.
// This is due to the render editable and editable text handle pointer
// event separately. If the editable text changes the text during the
// event handler, the render editable will use the outdated text stored in
// the _plainText when handling the pointer event.
//
// If this happens, we need to make sure the new selection is still valid.
final int textLength = textSelectionDelegate.textEditingValue.text.length;
nextSelection = nextSelection.copyWith(
baseOffset: math.min(nextSelection.baseOffset, textLength),
extentOffset: math.min(nextSelection.extentOffset, textLength),
);
}
_handleSelectionChange(nextSelection, cause); _handleSelectionChange(nextSelection, cause);
_setTextEditingValue( _setTextEditingValue(
textSelectionDelegate.textEditingValue.copyWith(selection: nextSelection), textSelectionDelegate.textEditingValue.copyWith(selection: nextSelection),
......
...@@ -444,6 +444,33 @@ void main() { ...@@ -444,6 +444,33 @@ void main() {
expect(renderEditable.cursorColor!.alpha, 0); expect(renderEditable.cursorColor!.alpha, 0);
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS })); }, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS }));
// Regression test for https://github.com/flutter/flutter/issues/78918.
testWidgets('RenderEditable sets correct text editing value', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController(text: 'how are you');
final UniqueKey icon = UniqueKey();
await tester.pumpWidget(
MaterialApp(
home: Material(
child: TextField(
controller: controller,
decoration: InputDecoration(
suffixIcon: IconButton(
key: icon,
icon: const Icon(Icons.cancel),
onPressed: () => controller.clear(),
),
),
),
),
),
);
await tester.tap(find.byKey(icon));
await tester.pump();
expect(controller.text, '');
expect(controller.selection, const TextSelection.collapsed(offset: 0, affinity: TextAffinity.upstream));
});
testWidgets('Cursor radius is 2.0', (WidgetTester tester) async { testWidgets('Cursor radius is 2.0', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
const MaterialApp( const MaterialApp(
......
...@@ -413,7 +413,9 @@ void main() { ...@@ -413,7 +413,9 @@ void main() {
}); });
test('selects correct place with offsets', () { test('selects correct place with offsets', () {
final TextSelectionDelegate delegate = FakeEditableTextState(); const String text = 'test\ntest';
final TextSelectionDelegate delegate = FakeEditableTextState()
..textEditingValue = const TextEditingValue(text: text);
final ViewportOffset viewportOffset = ViewportOffset.zero(); final ViewportOffset viewportOffset = ViewportOffset.zero();
late TextSelection currentSelection; late TextSelection currentSelection;
final RenderEditable editable = RenderEditable( final RenderEditable editable = RenderEditable(
...@@ -431,7 +433,7 @@ void main() { ...@@ -431,7 +433,7 @@ void main() {
startHandleLayerLink: LayerLink(), startHandleLayerLink: LayerLink(),
endHandleLayerLink: LayerLink(), endHandleLayerLink: LayerLink(),
text: const TextSpan( text: const TextSpan(
text: 'test\ntest', text: text,
style: TextStyle( style: TextStyle(
height: 1.0, fontSize: 10.0, fontFamily: 'Ahem', height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
), ),
...@@ -498,7 +500,9 @@ void main() { ...@@ -498,7 +500,9 @@ void main() {
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/61026 }, skip: isBrowser); // https://github.com/flutter/flutter/issues/61026
test('selects correct place when offsets are flipped', () { test('selects correct place when offsets are flipped', () {
final TextSelectionDelegate delegate = FakeEditableTextState(); const String text = 'abc def ghi';
final TextSelectionDelegate delegate = FakeEditableTextState()
..textEditingValue = const TextEditingValue(text: text);
final ViewportOffset viewportOffset = ViewportOffset.zero(); final ViewportOffset viewportOffset = ViewportOffset.zero();
late TextSelection currentSelection; late TextSelection currentSelection;
final RenderEditable editable = RenderEditable( final RenderEditable editable = RenderEditable(
...@@ -512,7 +516,7 @@ void main() { ...@@ -512,7 +516,7 @@ void main() {
currentSelection = selection; currentSelection = selection;
}, },
text: const TextSpan( text: const TextSpan(
text: 'abc def ghi', text: text,
style: TextStyle( style: TextStyle(
height: 1.0, fontSize: 10.0, fontFamily: 'Ahem', height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
), ),
...@@ -534,9 +538,11 @@ void main() { ...@@ -534,9 +538,11 @@ void main() {
test('selection does not flicker as user is dragging', () { test('selection does not flicker as user is dragging', () {
int selectionChangedCount = 0; int selectionChangedCount = 0;
TextSelection? updatedSelection; TextSelection? updatedSelection;
final TextSelectionDelegate delegate = FakeEditableTextState(); const String text = 'abc def ghi';
const TextSpan text = TextSpan( final TextSelectionDelegate delegate = FakeEditableTextState()
text: 'abc def ghi', ..textEditingValue = const TextEditingValue(text: text);
const TextSpan span = TextSpan(
text: text,
style: TextStyle( style: TextStyle(
height: 1.0, fontSize: 10.0, fontFamily: 'Ahem', height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
), ),
...@@ -553,7 +559,7 @@ void main() { ...@@ -553,7 +559,7 @@ void main() {
}, },
startHandleLayerLink: LayerLink(), startHandleLayerLink: LayerLink(),
endHandleLayerLink: LayerLink(), endHandleLayerLink: LayerLink(),
text: text, text: span,
); );
layout(editable1); layout(editable1);
...@@ -574,7 +580,7 @@ void main() { ...@@ -574,7 +580,7 @@ void main() {
selectionChangedCount++; selectionChangedCount++;
updatedSelection = selection; updatedSelection = selection;
}, },
text: text, text: span,
startHandleLayerLink: LayerLink(), startHandleLayerLink: LayerLink(),
endHandleLayerLink: LayerLink(), endHandleLayerLink: LayerLink(),
); );
...@@ -933,8 +939,10 @@ void main() { ...@@ -933,8 +939,10 @@ void main() {
expect(delegate.textEditingValue.text, '01232345'); expect(delegate.textEditingValue.text, '01232345');
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/61021 }, skip: isBrowser); // https://github.com/flutter/flutter/issues/61021
test('arrow keys and delete handle surrogate pairs correctly', () async { test('arrow keys and delete handle surrogate pairs correctly case 2', () async {
final TextSelectionDelegate delegate = FakeEditableTextState(); const String text = '\u{1F44D}';
final TextSelectionDelegate delegate = FakeEditableTextState()
..textEditingValue = const TextEditingValue(text: text);
final ViewportOffset viewportOffset = ViewportOffset.zero(); final ViewportOffset viewportOffset = ViewportOffset.zero();
late TextSelection currentSelection; late TextSelection currentSelection;
final RenderEditable editable = RenderEditable( final RenderEditable editable = RenderEditable(
...@@ -951,7 +959,7 @@ void main() { ...@@ -951,7 +959,7 @@ void main() {
startHandleLayerLink: LayerLink(), startHandleLayerLink: LayerLink(),
endHandleLayerLink: LayerLink(), endHandleLayerLink: LayerLink(),
text: const TextSpan( text: const TextSpan(
text: '\u{1F44D}', // Thumbs up text: text, // Thumbs up
style: TextStyle( style: TextStyle(
height: 1.0, fontSize: 10.0, fontFamily: 'Ahem', height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
), ),
...@@ -1038,7 +1046,9 @@ void main() { ...@@ -1038,7 +1046,9 @@ void main() {
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/61021 }, skip: isBrowser); // https://github.com/flutter/flutter/issues/61021
test('arrow keys with selection text', () async { test('arrow keys with selection text', () async {
final TextSelectionDelegate delegate = FakeEditableTextState(); const String text = '012345';
final TextSelectionDelegate delegate = FakeEditableTextState()
..textEditingValue = const TextEditingValue(text: text);
final ViewportOffset viewportOffset = ViewportOffset.zero(); final ViewportOffset viewportOffset = ViewportOffset.zero();
late TextSelection currentSelection; late TextSelection currentSelection;
final RenderEditable editable = RenderEditable( final RenderEditable editable = RenderEditable(
...@@ -1055,7 +1065,7 @@ void main() { ...@@ -1055,7 +1065,7 @@ void main() {
startHandleLayerLink: LayerLink(), startHandleLayerLink: LayerLink(),
endHandleLayerLink: LayerLink(), endHandleLayerLink: LayerLink(),
text: const TextSpan( text: const TextSpan(
text: '012345', // Thumbs up text: text, // Thumbs up
style: TextStyle(height: 1.0, fontSize: 10.0, fontFamily: 'Ahem'), style: TextStyle(height: 1.0, fontSize: 10.0, fontFamily: 'Ahem'),
), ),
selection: const TextSelection.collapsed( selection: const TextSelection.collapsed(
...@@ -1096,7 +1106,9 @@ void main() { ...@@ -1096,7 +1106,9 @@ void main() {
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/58068 }, skip: isBrowser); // https://github.com/flutter/flutter/issues/58068
test('arrow keys with selection text and shift', () async { test('arrow keys with selection text and shift', () async {
final TextSelectionDelegate delegate = FakeEditableTextState(); const String text = '012345';
final TextSelectionDelegate delegate = FakeEditableTextState()
..textEditingValue = const TextEditingValue(text: text);
final ViewportOffset viewportOffset = ViewportOffset.zero(); final ViewportOffset viewportOffset = ViewportOffset.zero();
late TextSelection currentSelection; late TextSelection currentSelection;
final RenderEditable editable = RenderEditable( final RenderEditable editable = RenderEditable(
...@@ -1113,7 +1125,7 @@ void main() { ...@@ -1113,7 +1125,7 @@ void main() {
startHandleLayerLink: LayerLink(), startHandleLayerLink: LayerLink(),
endHandleLayerLink: LayerLink(), endHandleLayerLink: LayerLink(),
text: const TextSpan( text: const TextSpan(
text: '012345', // Thumbs up text: text, // Thumbs up
style: TextStyle(height: 1.0, fontSize: 10.0, fontFamily: 'Ahem'), style: TextStyle(height: 1.0, fontSize: 10.0, fontFamily: 'Ahem'),
), ),
selection: const TextSelection.collapsed( selection: const TextSelection.collapsed(
...@@ -1158,7 +1170,9 @@ void main() { ...@@ -1158,7 +1170,9 @@ void main() {
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/58068 }, skip: isBrowser); // https://github.com/flutter/flutter/issues/58068
test('respects enableInteractiveSelection', () async { test('respects enableInteractiveSelection', () async {
final TextSelectionDelegate delegate = FakeEditableTextState(); const String text = '012345';
final TextSelectionDelegate delegate = FakeEditableTextState()
..textEditingValue = const TextEditingValue(text: text);
final ViewportOffset viewportOffset = ViewportOffset.zero(); final ViewportOffset viewportOffset = ViewportOffset.zero();
late TextSelection currentSelection; late TextSelection currentSelection;
final RenderEditable editable = RenderEditable( final RenderEditable editable = RenderEditable(
...@@ -1175,7 +1189,7 @@ void main() { ...@@ -1175,7 +1189,7 @@ void main() {
startHandleLayerLink: LayerLink(), startHandleLayerLink: LayerLink(),
endHandleLayerLink: LayerLink(), endHandleLayerLink: LayerLink(),
text: const TextSpan( text: const TextSpan(
text: '012345', // Thumbs up text: text, // Thumbs up
style: TextStyle(height: 1.0, fontSize: 10.0, fontFamily: 'Ahem'), style: TextStyle(height: 1.0, fontSize: 10.0, fontFamily: 'Ahem'),
), ),
selection: const TextSelection.collapsed( selection: const TextSelection.collapsed(
......
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