From e3097cdd289c9cc89275d74d722bc9ab1a6f6dc0 Mon Sep 17 00:00:00 2001 From: Hassan Toor Date: Mon, 11 Mar 2024 11:52:12 -0500 Subject: [PATCH 1/3] Fix composing text interruption on geometry updates --- .../src/engine/text_editing/text_editing.dart | 11 ++- lib/web_ui/test/engine/text_editing_test.dart | 70 +++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 813885329be1d..19b5dba1e15a6 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -1338,7 +1338,16 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements void updateElementPlacement(EditableTextGeometry textGeometry) { geometry = textGeometry; if (isEnabled) { - placeElement(); + // On updates, we shouldn't go through the entire placeElement() flow if + // we are in the middle of IME composition, otherwise we risk interrupting it. + // Geometry updates occur when a multiline input expands or contracts. If + // we are in the middle of composition, we should just update the geometry. + // See: https://github.com/flutter/flutter/issues/98817 + if(composingText != null) { + geometry?.applyToDomElement(activeDomElement); + } else { + placeElement(); + } } } diff --git a/lib/web_ui/test/engine/text_editing_test.dart b/lib/web_ui/test/engine/text_editing_test.dart index 09a99a242d964..0c644655bed95 100644 --- a/lib/web_ui/test/engine/text_editing_test.dart +++ b/lib/web_ui/test/engine/text_editing_test.dart @@ -513,6 +513,64 @@ Future testMain() async { expect(editingStrategy!.domElement!.style.width, '13px'); expect(editingStrategy!.domElement!.style.height, '12px'); }); + + test('updateElementPlacement() should not call placeElement() when in mid-composition', () { + final HybridTextEditing testTextEditing = HybridTextEditing(); + final GlobalTextEditingStrategySpy editingStrategy = GlobalTextEditingStrategySpy(testTextEditing); + testTextEditing.debugTextEditingStrategyOverride = editingStrategy; + testTextEditing.configuration = singlelineConfig; + + editingStrategy.enable( + singlelineConfig, + onChange: trackEditingState, + onAction: trackInputAction, + ); + expect(editingStrategy.isEnabled, isTrue); + + // placeElement() called from enable() + expect(editingStrategy.placeElementCount, 1); + + // No geometry should be set until setEditableSizeAndTransform is called. + expect(editingStrategy.domElement!.style.transform, ''); + expect(editingStrategy.domElement!.style.width, ''); + expect(editingStrategy.domElement!.style.height, ''); + + // set some composing text. + editingStrategy.composingText = '뮤'; + + testTextEditing.acceptCommand(TextInputSetEditableSizeAndTransform(geometry: EditableTextGeometry( + width: 13, + height: 12, + globalTransform: Matrix4.translationValues(14, 15, 0).storage, + )), () {}); + + // placeElement() should not be called again. + expect(editingStrategy.placeElementCount, 1); + + // geometry should be applied. + expect(editingStrategy.domElement!.style.transform, + 'matrix(1, 0, 0, 1, 14, 15)'); + expect(editingStrategy.domElement!.style.width, '13px'); + expect(editingStrategy.domElement!.style.height, '12px'); + + // set composing text to null. + editingStrategy.composingText = null; + + testTextEditing.acceptCommand(TextInputSetEditableSizeAndTransform(geometry: EditableTextGeometry( + width: 10, + height: 10, + globalTransform: Matrix4.translationValues(11, 12, 0).storage, + )), () {}); + + // placeElement() should be called again. + expect(editingStrategy.placeElementCount, 2); + + // geometry should be updated. + expect(editingStrategy.domElement!.style.transform, + 'matrix(1, 0, 0, 1, 11, 12)'); + expect(editingStrategy.domElement!.style.width, '10px'); + expect(editingStrategy.domElement!.style.height, '10px'); + }); }); group('$HybridTextEditing', () { @@ -3408,3 +3466,15 @@ Future waitForDesktopSafariFocus() async { await Future.delayed(Duration.zero); } } + +class GlobalTextEditingStrategySpy extends GloballyPositionedTextEditingStrategy { + GlobalTextEditingStrategySpy(super.owner); + + int placeElementCount = 0; + + @override + void placeElement() { + placeElementCount++; + super.placeElement(); + } +} From 2b94b07e9df62d1050fefbecf6470c4cd7dda279 Mon Sep 17 00:00:00 2001 From: Hassan Toor Date: Mon, 11 Mar 2024 11:53:33 -0500 Subject: [PATCH 2/3] Linting --- lib/web_ui/lib/src/engine/text_editing/text_editing.dart | 4 ++-- lib/web_ui/test/engine/text_editing_test.dart | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 19b5dba1e15a6..495ad8c89fa4b 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -1338,9 +1338,9 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements void updateElementPlacement(EditableTextGeometry textGeometry) { geometry = textGeometry; if (isEnabled) { - // On updates, we shouldn't go through the entire placeElement() flow if + // On updates, we shouldn't go through the entire placeElement() flow if // we are in the middle of IME composition, otherwise we risk interrupting it. - // Geometry updates occur when a multiline input expands or contracts. If + // Geometry updates occur when a multiline input expands or contracts. If // we are in the middle of composition, we should just update the geometry. // See: https://github.com/flutter/flutter/issues/98817 if(composingText != null) { diff --git a/lib/web_ui/test/engine/text_editing_test.dart b/lib/web_ui/test/engine/text_editing_test.dart index 0c644655bed95..a9fa99443eb5b 100644 --- a/lib/web_ui/test/engine/text_editing_test.dart +++ b/lib/web_ui/test/engine/text_editing_test.dart @@ -3471,7 +3471,7 @@ class GlobalTextEditingStrategySpy extends GloballyPositionedTextEditingStrategy GlobalTextEditingStrategySpy(super.owner); int placeElementCount = 0; - + @override void placeElement() { placeElementCount++; From 7e725669a3a4fbbd1c28f30723f9807be2defa25 Mon Sep 17 00:00:00 2001 From: Hassan Toor Date: Wed, 13 Mar 2024 09:14:18 -0500 Subject: [PATCH 3/3] spacing --- lib/web_ui/lib/src/engine/text_editing/text_editing.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 495ad8c89fa4b..672a5ad3be61d 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -1343,7 +1343,7 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements // Geometry updates occur when a multiline input expands or contracts. If // we are in the middle of composition, we should just update the geometry. // See: https://github.com/flutter/flutter/issues/98817 - if(composingText != null) { + if (composingText != null) { geometry?.applyToDomElement(activeDomElement); } else { placeElement();