Skip to content

Commit 03dbf1a

Browse files
koji-1009mdebbar
andauthored
[Web][Engine] Fix composingBaseOffset and composingExtentOffset value when input japanese text (flutter#161593)
fix flutter#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]. <!-- Links --> [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 <[email protected]>
1 parent cf007b9 commit 03dbf1a

File tree

4 files changed

+231
-7
lines changed

4 files changed

+231
-7
lines changed

engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ mixin CompositionAwareMixin {
4747
/// so it is safe to reference it to get the current composingText.
4848
String? composingText;
4949

50+
/// The base offset of the composing text in the `InputElement` or `TextAreaElement`.
51+
///
52+
/// Will be null if composing just started, ended, or no composing is being done.
53+
int? composingBase;
54+
5055
void addCompositionEventHandlers(DomHTMLElement domElement) {
5156
domElement.addEventListener(_kCompositionStart, _compositionStartListener);
5257
domElement.addEventListener(_kCompositionUpdate, _compositionUpdateListener);
@@ -61,6 +66,7 @@ mixin CompositionAwareMixin {
6166

6267
void _handleCompositionStart(DomEvent event) {
6368
composingText = null;
69+
composingBase = null;
6470
}
6571

6672
void _handleCompositionUpdate(DomEvent event) {
@@ -71,22 +77,22 @@ mixin CompositionAwareMixin {
7177

7278
void _handleCompositionEnd(DomEvent event) {
7379
composingText = null;
80+
composingBase = null;
7481
}
7582

7683
EditingState determineCompositionState(EditingState editingState) {
7784
if (composingText == null) {
7885
return editingState;
7986
}
8087

81-
final int composingBase = editingState.extentOffset - composingText!.length;
82-
83-
if (composingBase < 0) {
88+
composingBase ??= editingState.extentOffset - composingText!.length;
89+
if (composingBase! < 0) {
8490
return editingState;
8591
}
8692

8793
return editingState.copyWith(
8894
composingBaseOffset: composingBase,
89-
composingExtentOffset: composingBase + composingText!.length,
95+
composingExtentOffset: composingBase! + composingText!.length,
9096
);
9197
}
9298
}

engine/src/flutter/lib/web_ui/test/engine/composition_test.dart

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,35 @@ Future<void> testMain() async {
210210
),
211211
);
212212
});
213+
214+
test('should retain composing base offset if composing text area is changed', () {
215+
const String composingText = '今日は寒い日です';
216+
217+
EditingState editingState = EditingState(text: '今日は寒い日です', baseOffset: 0, extentOffset: 8);
218+
219+
final _MockWithCompositionAwareMixin mockWithCompositionAwareMixin =
220+
_MockWithCompositionAwareMixin();
221+
mockWithCompositionAwareMixin.composingText = composingText;
222+
223+
expect(
224+
mockWithCompositionAwareMixin.determineCompositionState(editingState),
225+
editingState.copyWith(composingBaseOffset: 0, composingExtentOffset: 8),
226+
);
227+
228+
editingState = editingState.copyWith(baseOffset: 0, extentOffset: 3);
229+
230+
expect(
231+
mockWithCompositionAwareMixin.determineCompositionState(editingState),
232+
editingState.copyWith(composingBaseOffset: 0, composingExtentOffset: 8),
233+
);
234+
235+
editingState = editingState.copyWith(baseOffset: 3, extentOffset: 6);
236+
237+
expect(
238+
mockWithCompositionAwareMixin.determineCompositionState(editingState),
239+
editingState.copyWith(composingBaseOffset: 0, composingExtentOffset: 8),
240+
);
241+
});
213242
});
214243
});
215244

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

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5574,7 +5574,10 @@ class EditableTextState extends State<EditableText>
55745574
),
55755575
),
55765576
ScrollToDocumentBoundaryIntent: _makeOverridable(
5577-
CallbackAction<ScrollToDocumentBoundaryIntent>(onInvoke: _scrollToDocumentBoundary),
5577+
_WebComposingDisablingCallbackAction<ScrollToDocumentBoundaryIntent>(
5578+
this,
5579+
onInvoke: _scrollToDocumentBoundary,
5580+
),
55785581
),
55795582
ScrollIntent: CallbackAction<ScrollIntent>(onInvoke: _scroll),
55805583

@@ -6482,7 +6485,13 @@ class _UpdateTextSelectionAction<T extends DirectionalCaretMovementIntent>
64826485
}
64836486

64846487
@override
6485-
bool get isActionEnabled => state._value.selection.isValid;
6488+
bool get isActionEnabled {
6489+
if (kIsWeb && state.widget.selectionEnabled && state._value.composing.isValid) {
6490+
return false;
6491+
}
6492+
6493+
return state._value.selection.isValid;
6494+
}
64866495
}
64876496

64886497
class _UpdateTextSelectionVerticallyAction<T extends DirectionalCaretMovementIntent>
@@ -6562,7 +6571,28 @@ class _UpdateTextSelectionVerticallyAction<T extends DirectionalCaretMovementInt
65626571
}
65636572

65646573
@override
6565-
bool get isActionEnabled => state._value.selection.isValid;
6574+
bool get isActionEnabled {
6575+
if (kIsWeb && state.widget.selectionEnabled && state._value.composing.isValid) {
6576+
return false;
6577+
}
6578+
6579+
return state._value.selection.isValid;
6580+
}
6581+
}
6582+
6583+
class _WebComposingDisablingCallbackAction<T extends Intent> extends CallbackAction<T> {
6584+
_WebComposingDisablingCallbackAction(this.state, {required super.onInvoke});
6585+
6586+
final EditableTextState state;
6587+
6588+
@override
6589+
bool get isActionEnabled {
6590+
if (kIsWeb && state.widget.selectionEnabled && state._value.composing.isValid) {
6591+
return false;
6592+
}
6593+
6594+
return super.isActionEnabled;
6595+
}
65666596
}
65676597

65686598
class _SelectAllAction extends ContextAction<SelectAllTextIntent> {

packages/flutter/test/widgets/editable_text_shortcuts_test.dart

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2864,4 +2864,163 @@ void main() {
28642864
);
28652865
});
28662866
}, skip: !kIsWeb); // [intended] specific tests target web.
2867+
2868+
group(
2869+
'Web does not accept',
2870+
() {
2871+
testWidgets('character modifier + arrowLeft in composing', (WidgetTester tester) async {
2872+
const SingleActivator arrowLeft = SingleActivator(
2873+
LogicalKeyboardKey.arrowLeft,
2874+
shift: true,
2875+
);
2876+
2877+
controller.value = const TextEditingValue(
2878+
text: testText,
2879+
selection: TextSelection(baseOffset: 0, extentOffset: 3),
2880+
composing: TextRange(start: 0, end: 3),
2881+
);
2882+
2883+
await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12)));
2884+
await tester.pumpAndSettle();
2885+
2886+
await sendKeyCombination(tester, arrowLeft);
2887+
await tester.pump();
2888+
2889+
// selection should not change.
2890+
expect(controller.text, testText);
2891+
expect(
2892+
controller.selection,
2893+
const TextSelection(baseOffset: 0, extentOffset: 3),
2894+
reason: arrowLeft.toString(),
2895+
);
2896+
});
2897+
2898+
testWidgets('character modifier + arrowRight in composing', (WidgetTester tester) async {
2899+
const SingleActivator arrowRight = SingleActivator(
2900+
LogicalKeyboardKey.arrowLeft,
2901+
shift: true,
2902+
);
2903+
2904+
controller.value = const TextEditingValue(
2905+
text: testText,
2906+
selection: TextSelection(baseOffset: 0, extentOffset: 3),
2907+
composing: TextRange(start: 0, end: 3),
2908+
);
2909+
2910+
await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12)));
2911+
await tester.pumpAndSettle();
2912+
2913+
await sendKeyCombination(tester, arrowRight);
2914+
await tester.pump();
2915+
2916+
// selection should not change.
2917+
expect(controller.text, testText);
2918+
expect(
2919+
controller.selection,
2920+
const TextSelection(baseOffset: 0, extentOffset: 3),
2921+
reason: arrowRight.toString(),
2922+
);
2923+
});
2924+
2925+
testWidgets('character modifier + arrowUp in composing', (WidgetTester tester) async {
2926+
const SingleActivator arrowUp = SingleActivator(LogicalKeyboardKey.arrowUp, shift: true);
2927+
2928+
controller.value = const TextEditingValue(
2929+
text: testText,
2930+
selection: TextSelection(baseOffset: 0, extentOffset: 3),
2931+
composing: TextRange(start: 0, end: 3),
2932+
);
2933+
2934+
await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12)));
2935+
await tester.pumpAndSettle();
2936+
2937+
await sendKeyCombination(tester, arrowUp);
2938+
await tester.pump();
2939+
2940+
// selection should not change.
2941+
expect(controller.text, testText);
2942+
expect(
2943+
controller.selection,
2944+
const TextSelection(baseOffset: 0, extentOffset: 3),
2945+
reason: arrowUp.toString(),
2946+
);
2947+
});
2948+
2949+
testWidgets('character modifier + arrowDown in composing', (WidgetTester tester) async {
2950+
const SingleActivator arrowDown = SingleActivator(
2951+
LogicalKeyboardKey.arrowDown,
2952+
shift: true,
2953+
);
2954+
2955+
controller.value = const TextEditingValue(
2956+
text: testText,
2957+
selection: TextSelection(baseOffset: 0, extentOffset: 3),
2958+
composing: TextRange(start: 0, end: 3),
2959+
);
2960+
2961+
await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12)));
2962+
await tester.pumpAndSettle();
2963+
2964+
await sendKeyCombination(tester, arrowDown);
2965+
await tester.pump();
2966+
2967+
// selection should not change.
2968+
expect(controller.text, testText);
2969+
expect(
2970+
controller.selection,
2971+
const TextSelection(baseOffset: 0, extentOffset: 3),
2972+
reason: arrowDown.toString(),
2973+
);
2974+
});
2975+
2976+
testWidgets('home in composing', (WidgetTester tester) async {
2977+
const SingleActivator home = SingleActivator(LogicalKeyboardKey.home);
2978+
2979+
controller.value = const TextEditingValue(
2980+
text: testText,
2981+
selection: TextSelection(baseOffset: 0, extentOffset: 3),
2982+
composing: TextRange(start: 0, end: 3),
2983+
);
2984+
2985+
await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12)));
2986+
await tester.pumpAndSettle();
2987+
2988+
await sendKeyCombination(tester, home);
2989+
await tester.pump();
2990+
2991+
// selection should not change.
2992+
expect(controller.text, testText);
2993+
expect(
2994+
controller.selection,
2995+
const TextSelection(baseOffset: 0, extentOffset: 3),
2996+
reason: home.toString(),
2997+
);
2998+
});
2999+
3000+
testWidgets('end in composing', (WidgetTester tester) async {
3001+
const SingleActivator end = SingleActivator(LogicalKeyboardKey.end);
3002+
3003+
controller.value = const TextEditingValue(
3004+
text: testText,
3005+
selection: TextSelection(baseOffset: 0, extentOffset: 3),
3006+
composing: TextRange(start: 0, end: 3),
3007+
);
3008+
3009+
await tester.pumpWidget(buildEditableText(style: const TextStyle(fontSize: 12)));
3010+
await tester.pumpAndSettle();
3011+
3012+
await sendKeyCombination(tester, end);
3013+
await tester.pump();
3014+
3015+
// selection should not change.
3016+
expect(controller.text, testText);
3017+
expect(
3018+
controller.selection,
3019+
const TextSelection(baseOffset: 0, extentOffset: 3),
3020+
reason: end.toString(),
3021+
);
3022+
});
3023+
},
3024+
skip: !kIsWeb, // [intended] specific tests target web.
3025+
);
28673026
}

0 commit comments

Comments
 (0)