Unverified Commit b851f997 authored by Chris Bracken's avatar Chris Bracken Committed by GitHub

Preserve composing range if possible on sel change (#67197)

When setting the TextSelection in a TextEditingController, preserve the
composing range so long as the new selection is:

  * a collapsed selection; in other words, a cursor rather than a
    selection with an extent. A selection with an extent is not
    permitted when composing.
  * within the composing region. Moving the cursor outside the composing
    region ends composing mode.

When using physical keyboards for input with an IME, users expect to be
able to navigate using the cursor within the composing region with the
arrow keys in order to edit text in the composing range.

As an example, a user might erroneously enter the composing text
にほんごにゅうろく in kana, then hit the left arrow, followed by
backspace in order to delete the ろ, then input りょ in order to
generate the correct composing text にほんごにゅうりょく, before then
hitting the conversion key to convert to the kanji text 日本語入力 and
commit.
parent 019e90f7
......@@ -214,10 +214,17 @@ class TextEditingController extends ValueNotifier<TextEditingValue> {
/// [TextEditingController]; however, one should not also set [text]
/// in a separate statement. To change both the [text] and the [selection]
/// change the controller's [value].
///
/// If the new selection if of non-zero length, or is outside the composing
/// range, the composing composing range is cleared.
set selection(TextSelection newSelection) {
if (!isSelectionWithinTextBounds(newSelection))
throw FlutterError('invalid text selection: $newSelection');
value = value.copyWith(selection: newSelection, composing: TextRange.empty);
final TextRange newComposing =
newSelection.isCollapsed && _isSelectionWithinComposingRange(newSelection)
? value.composing
: TextRange.empty;
value = value.copyWith(selection: newSelection, composing: newComposing);
}
/// Set the [value] to empty.
......@@ -251,6 +258,11 @@ class TextEditingController extends ValueNotifier<TextEditingValue> {
bool isSelectionWithinTextBounds(TextSelection selection) {
return selection.start <= text.length && selection.end <= text.length;
}
/// Check that the [selection] is inside of the composing range.
bool _isSelectionWithinComposingRange(TextSelection selection) {
return selection.start >= value.composing.start && selection.end <= value.composing.end;
}
}
/// Toolbar configuration for [EditableText].
......
......@@ -5706,6 +5706,119 @@ void main() {
expectToAssert(const TextEditingValue(text: 'test', composing: TextRange(start: -1, end: 9)), false);
});
testWidgets('Preserves composing range if cursor moves within that range', (WidgetTester tester) async {
final Widget widget = MaterialApp(
home: EditableText(
backgroundCursorColor: Colors.grey,
controller: controller,
focusNode: focusNode,
style: textStyle,
cursorColor: cursorColor,
selectionControls: materialTextSelectionControls,
),
);
await tester.pumpWidget(widget);
final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText));
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
composing: TextRange(start: 4, end: 12),
));
controller.selection = const TextSelection.collapsed(offset: 5);
expect(state.currentTextEditingValue.composing, const TextRange(start: 4, end: 12));
});
testWidgets('Clears composing range if cursor moves outside that range', (WidgetTester tester) async {
final Widget widget = MaterialApp(
home: EditableText(
backgroundCursorColor: Colors.grey,
controller: controller,
focusNode: focusNode,
style: textStyle,
cursorColor: cursorColor,
selectionControls: materialTextSelectionControls,
),
);
await tester.pumpWidget(widget);
// Positioning cursor before the composing range should clear the composing range.
final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText));
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
composing: TextRange(start: 4, end: 12),
));
controller.selection = const TextSelection.collapsed(offset: 2);
expect(state.currentTextEditingValue.composing, TextRange.empty);
// Reset the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
composing: TextRange(start: 4, end: 12),
));
expect(state.currentTextEditingValue.composing, const TextRange(start: 4, end: 12));
// Positioning cursor after the composing range should clear the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
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 {
final Widget widget = MaterialApp(
home: EditableText(
backgroundCursorColor: Colors.grey,
controller: controller,
focusNode: focusNode,
style: textStyle,
cursorColor: cursorColor,
selectionControls: materialTextSelectionControls,
),
);
await tester.pumpWidget(widget);
// Setting a selection before the composing range clears the composing range.
final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText));
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
composing: TextRange(start: 4, end: 12),
));
controller.selection = const TextSelection(baseOffset: 1, extentOffset: 2);
expect(state.currentTextEditingValue.composing, TextRange.empty);
// Reset the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
composing: TextRange(start: 4, end: 12),
));
expect(state.currentTextEditingValue.composing, const TextRange(start: 4, end: 12));
// Setting a selection within the composing range clears the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
composing: TextRange(start: 4, end: 12),
));
controller.selection = const TextSelection(baseOffset: 5, extentOffset: 7);
expect(state.currentTextEditingValue.composing, TextRange.empty);
// Reset the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
composing: TextRange(start: 4, end: 12),
));
expect(state.currentTextEditingValue.composing, const TextRange(start: 4, end: 12));
// Setting a selection after the composing range clears the composing range.
state.updateEditingValue(const TextEditingValue(
text: 'foo composing bar',
composing: TextRange(start: 4, end: 12),
));
controller.selection = const TextSelection(baseOffset: 13, extentOffset: 15);
expect(state.currentTextEditingValue.composing, TextRange.empty);
});
// Regression test for https://github.com/flutter/flutter/issues/65374.
testWidgets('Length formatter will not cause crash while the TextEditingValue is composing', (WidgetTester tester) async {
final TextInputFormatter formatter = LengthLimitingTextInputFormatter(5);
......
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