From 03dbf1a99c0a9d845eeba8dd26eb4e8bbcedbc49 Mon Sep 17 00:00:00 2001 From: Koji Wakamiya Date: Wed, 4 Jun 2025 00:02:06 +0900 Subject: [PATCH] [Web][Engine] Fix composingBaseOffset and composingExtentOffset value when input japanese text (#161593) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix https://github.com/flutter/flutter/issues/159671 When entering Japanese text and operating `shift + ← || → || ↑ || ↓` while composing a character, `setSelectionRange` set (0,0) and the composing text is disappeared. For this reason, disable shit + arrow text shortcuts on web platform. ### Movie fixed https://github.com/user-attachments/assets/ad0bd199-92a5-4e1f-9f26-0c23981c013d master branch https://github.com/user-attachments/assets/934f256e-189b-4916-bb91-a49be60f17b3 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Mouad Debbar --- .../text_editing/composition_aware_mixin.dart | 14 +- .../web_ui/test/engine/composition_test.dart | 29 ++++ .../lib/src/widgets/editable_text.dart | 36 +++- .../widgets/editable_text_shortcuts_test.dart | 159 ++++++++++++++++++ 4 files changed, 231 insertions(+), 7 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart index 6341eebe779..12f123a53d1 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart @@ -47,6 +47,11 @@ mixin CompositionAwareMixin { /// so it is safe to reference it to get the current composingText. String? composingText; + /// The base offset of the composing text in the `InputElement` or `TextAreaElement`. + /// + /// Will be null if composing just started, ended, or no composing is being done. + int? composingBase; + void addCompositionEventHandlers(DomHTMLElement domElement) { domElement.addEventListener(_kCompositionStart, _compositionStartListener); domElement.addEventListener(_kCompositionUpdate, _compositionUpdateListener); @@ -61,6 +66,7 @@ mixin CompositionAwareMixin { void _handleCompositionStart(DomEvent event) { composingText = null; + composingBase = null; } void _handleCompositionUpdate(DomEvent event) { @@ -71,6 +77,7 @@ mixin CompositionAwareMixin { void _handleCompositionEnd(DomEvent event) { composingText = null; + composingBase = null; } EditingState determineCompositionState(EditingState editingState) { @@ -78,15 +85,14 @@ mixin CompositionAwareMixin { return editingState; } - final int composingBase = editingState.extentOffset - composingText!.length; - - if (composingBase < 0) { + composingBase ??= editingState.extentOffset - composingText!.length; + if (composingBase! < 0) { return editingState; } return editingState.copyWith( composingBaseOffset: composingBase, - composingExtentOffset: composingBase + composingText!.length, + composingExtentOffset: composingBase! + composingText!.length, ); } } diff --git a/engine/src/flutter/lib/web_ui/test/engine/composition_test.dart b/engine/src/flutter/lib/web_ui/test/engine/composition_test.dart index c75e34d072f..d5bcebc4b06 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/composition_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/composition_test.dart @@ -210,6 +210,35 @@ Future testMain() async { ), ); }); + + test('should retain composing base offset if composing text area is changed', () { + const String composingText = '今日は寒い日です'; + + EditingState editingState = EditingState(text: '今日は寒い日です', baseOffset: 0, extentOffset: 8); + + final _MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + _MockWithCompositionAwareMixin(); + mockWithCompositionAwareMixin.composingText = composingText; + + expect( + mockWithCompositionAwareMixin.determineCompositionState(editingState), + editingState.copyWith(composingBaseOffset: 0, composingExtentOffset: 8), + ); + + editingState = editingState.copyWith(baseOffset: 0, extentOffset: 3); + + expect( + mockWithCompositionAwareMixin.determineCompositionState(editingState), + editingState.copyWith(composingBaseOffset: 0, composingExtentOffset: 8), + ); + + editingState = editingState.copyWith(baseOffset: 3, extentOffset: 6); + + expect( + mockWithCompositionAwareMixin.determineCompositionState(editingState), + editingState.copyWith(composingBaseOffset: 0, composingExtentOffset: 8), + ); + }); }); }); diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 6315f6843eb..1a96c8ffc38 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -5574,7 +5574,10 @@ class EditableTextState extends State ), ), ScrollToDocumentBoundaryIntent: _makeOverridable( - CallbackAction(onInvoke: _scrollToDocumentBoundary), + _WebComposingDisablingCallbackAction( + this, + onInvoke: _scrollToDocumentBoundary, + ), ), ScrollIntent: CallbackAction(onInvoke: _scroll), @@ -6482,7 +6485,13 @@ class _UpdateTextSelectionAction } @override - bool get isActionEnabled => state._value.selection.isValid; + bool get isActionEnabled { + if (kIsWeb && state.widget.selectionEnabled && state._value.composing.isValid) { + return false; + } + + return state._value.selection.isValid; + } } class _UpdateTextSelectionVerticallyAction @@ -6562,7 +6571,28 @@ class _UpdateTextSelectionVerticallyAction state._value.selection.isValid; + bool get isActionEnabled { + if (kIsWeb && state.widget.selectionEnabled && state._value.composing.isValid) { + return false; + } + + return state._value.selection.isValid; + } +} + +class _WebComposingDisablingCallbackAction extends CallbackAction { + _WebComposingDisablingCallbackAction(this.state, {required super.onInvoke}); + + final EditableTextState state; + + @override + bool get isActionEnabled { + if (kIsWeb && state.widget.selectionEnabled && state._value.composing.isValid) { + return false; + } + + return super.isActionEnabled; + } } class _SelectAllAction extends ContextAction { diff --git a/packages/flutter/test/widgets/editable_text_shortcuts_test.dart b/packages/flutter/test/widgets/editable_text_shortcuts_test.dart index fe49f4e7bf4..d88e0e201dc 100644 --- a/packages/flutter/test/widgets/editable_text_shortcuts_test.dart +++ b/packages/flutter/test/widgets/editable_text_shortcuts_test.dart @@ -2864,4 +2864,163 @@ void main() { ); }); }, skip: !kIsWeb); // [intended] specific tests target web. + + group( + 'Web does not accept', + () { + testWidgets('character modifier + arrowLeft in composing', (WidgetTester tester) async { + const SingleActivator arrowLeft = SingleActivator( + LogicalKeyboardKey.arrowLeft, + shift: true, + ); + + controller.value = const TextEditingValue( + text: testText, + selection: TextSelection(baseOffset: 0, extentOffset: 3), + composing: TextRange(start: 0, end: 3), + ); + + await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12))); + await tester.pumpAndSettle(); + + await sendKeyCombination(tester, arrowLeft); + await tester.pump(); + + // selection should not change. + expect(controller.text, testText); + expect( + controller.selection, + const TextSelection(baseOffset: 0, extentOffset: 3), + reason: arrowLeft.toString(), + ); + }); + + testWidgets('character modifier + arrowRight in composing', (WidgetTester tester) async { + const SingleActivator arrowRight = SingleActivator( + LogicalKeyboardKey.arrowLeft, + shift: true, + ); + + controller.value = const TextEditingValue( + text: testText, + selection: TextSelection(baseOffset: 0, extentOffset: 3), + composing: TextRange(start: 0, end: 3), + ); + + await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12))); + await tester.pumpAndSettle(); + + await sendKeyCombination(tester, arrowRight); + await tester.pump(); + + // selection should not change. + expect(controller.text, testText); + expect( + controller.selection, + const TextSelection(baseOffset: 0, extentOffset: 3), + reason: arrowRight.toString(), + ); + }); + + testWidgets('character modifier + arrowUp in composing', (WidgetTester tester) async { + const SingleActivator arrowUp = SingleActivator(LogicalKeyboardKey.arrowUp, shift: true); + + controller.value = const TextEditingValue( + text: testText, + selection: TextSelection(baseOffset: 0, extentOffset: 3), + composing: TextRange(start: 0, end: 3), + ); + + await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12))); + await tester.pumpAndSettle(); + + await sendKeyCombination(tester, arrowUp); + await tester.pump(); + + // selection should not change. + expect(controller.text, testText); + expect( + controller.selection, + const TextSelection(baseOffset: 0, extentOffset: 3), + reason: arrowUp.toString(), + ); + }); + + testWidgets('character modifier + arrowDown in composing', (WidgetTester tester) async { + const SingleActivator arrowDown = SingleActivator( + LogicalKeyboardKey.arrowDown, + shift: true, + ); + + controller.value = const TextEditingValue( + text: testText, + selection: TextSelection(baseOffset: 0, extentOffset: 3), + composing: TextRange(start: 0, end: 3), + ); + + await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12))); + await tester.pumpAndSettle(); + + await sendKeyCombination(tester, arrowDown); + await tester.pump(); + + // selection should not change. + expect(controller.text, testText); + expect( + controller.selection, + const TextSelection(baseOffset: 0, extentOffset: 3), + reason: arrowDown.toString(), + ); + }); + + testWidgets('home in composing', (WidgetTester tester) async { + const SingleActivator home = SingleActivator(LogicalKeyboardKey.home); + + controller.value = const TextEditingValue( + text: testText, + selection: TextSelection(baseOffset: 0, extentOffset: 3), + composing: TextRange(start: 0, end: 3), + ); + + await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12))); + await tester.pumpAndSettle(); + + await sendKeyCombination(tester, home); + await tester.pump(); + + // selection should not change. + expect(controller.text, testText); + expect( + controller.selection, + const TextSelection(baseOffset: 0, extentOffset: 3), + reason: home.toString(), + ); + }); + + testWidgets('end in composing', (WidgetTester tester) async { + const SingleActivator end = SingleActivator(LogicalKeyboardKey.end); + + controller.value = const TextEditingValue( + text: testText, + selection: TextSelection(baseOffset: 0, extentOffset: 3), + composing: TextRange(start: 0, end: 3), + ); + + await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12))); + await tester.pumpAndSettle(); + + await sendKeyCombination(tester, end); + await tester.pump(); + + // selection should not change. + expect(controller.text, testText); + expect( + controller.selection, + const TextSelection(baseOffset: 0, extentOffset: 3), + reason: end.toString(), + ); + }); + }, + skip: !kIsWeb, // [intended] specific tests target web. + ); }