From a7aa66164e3aaf33c99282dfcc26e877748ced4c Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Thu, 17 Oct 2019 15:11:44 -0700 Subject: [PATCH] Re-implement hardware keyboard text selection. (#42879) This re-implements keyboard text selection so that it will work on platforms other than Android (e.g. macOS, Linux, etc.). Also, fixed a number of bugs in editing selection via a hardware keyboard (unable to select backwards, incorrect conversion to ASCII when cutting to clipboard, lack of support for CTRL-SHIFT-ARROW word selection, etc.). Did not address the keyboard locale issues that remain, or add platform specific switches for the bindings. All that will need some more design work before implementing them. Related Issues Fixes #31951 --- .../flutter/lib/src/rendering/editable.dart | 517 +++++++++--------- .../lib/src/services/keyboard_key.dart | 15 + .../test/material/text_field_test.dart | 23 +- .../flutter/test/rendering/editable_test.dart | 2 +- .../test/services/keyboard_key_test.dart | 46 ++ .../test/widgets/editable_text_test.dart | 381 ++++++++++++- .../test/widgets/selectable_text_test.dart | 16 +- 7 files changed, 706 insertions(+), 294 deletions(-) diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart index 2ad116a48e..879da3ed8f 100644 --- a/packages/flutter/lib/src/rendering/editable.dart +++ b/packages/flutter/lib/src/rendering/editable.dart @@ -240,6 +240,8 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin { static const String obscuringCharacter = '•'; /// Called when the selection changes. + /// + /// If this is null, then selection changes will be ignored. SelectionChangedHandler onSelectionChanged; double _textLayoutLastMaxWidth; @@ -350,261 +352,239 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin { .contains(endOffset + effectiveOffset); } - static const int _kLeftArrowCode = 21; - static const int _kRightArrowCode = 22; - static const int _kUpArrowCode = 19; - static const int _kDownArrowCode = 20; - static const int _kXKeyCode = 52; - static const int _kCKeyCode = 31; - static const int _kVKeyCode = 50; - static const int _kAKeyCode = 29; - static const int _kDelKeyCode = 112; - - // The extent offset of the current selection - int _extentOffset = -1; - - // The base offset of the current selection - int _baseOffset = -1; - - // Holds the last location the user selected in the case that he selects all - // the way to the end or beginning of the field. - int _previousCursorLocation = -1; + // Holds the last cursor location the user selected in the case the user tries + // to select vertically past the end or beginning of the field. If they do, + // then we need to keep the old cursor location so that we can go back to it + // if they change their minds. Only used for moving selection up and down in a + // multi-line text field when selecting using the keyboard. + int _cursorResetLocation = -1; // Whether we should reset the location of the cursor in the case the user - // selects all the way to the end or the beginning of a field. - bool _resetCursor = false; + // tries to select vertically past the end or beginning of the field. If they + // do, then we need to keep the old cursor location so that we can go back to + // it if they change their minds. Only used for resetting selection up and + // down in a multi-line text field when selecting using the keyboard. + bool _wasSelectingVerticallyWithKeyboard = false; - static const int _kShiftMask = 1; // https://developer.android.com/reference/android/view/KeyEvent.html#META_SHIFT_ON - static const int _kControlMask = 1 << 12; // https://developer.android.com/reference/android/view/KeyEvent.html#META_CTRL_ON - - // Call through to onSelectionChanged only if the given nextSelection is - // different from the existing selection. - void _handlePotentialSelectionChange( + // Call through to onSelectionChanged. + void _handleSelectionChange( TextSelection nextSelection, SelectionChangedCause cause, ) { - if (nextSelection == selection) { + // Changes made by the keyboard can sometimes be "out of band" for listening + // components, so always send those events, even if we didn't think it + // changed. + if (nextSelection == selection && cause != SelectionChangedCause.keyboard) { return; } - onSelectionChanged(nextSelection, this, cause); + if (onSelectionChanged != null) { + onSelectionChanged(nextSelection, this, cause); + } } + static final Set _movementKeys = { + LogicalKeyboardKey.arrowRight, + LogicalKeyboardKey.arrowLeft, + LogicalKeyboardKey.arrowUp, + LogicalKeyboardKey.arrowDown, + }; + + static final Set _shortcutKeys = { + LogicalKeyboardKey.keyA, + LogicalKeyboardKey.keyC, + LogicalKeyboardKey.keyV, + LogicalKeyboardKey.keyX, + LogicalKeyboardKey.delete, + }; + + static final Set _nonModifierKeys = { + ..._shortcutKeys, + ..._movementKeys, + }; + + static final Set _modifierKeys = { + LogicalKeyboardKey.shift, + LogicalKeyboardKey.control, + }; + + static final Set _interestingKeys = { + ..._modifierKeys, + ..._nonModifierKeys, + }; + // TODO(goderbauer): doesn't handle extended grapheme clusters with more than one Unicode scalar value (https://github.com/flutter/flutter/issues/13404). + // This is because some of this code depends upon counting the length of the + // string using Unicode scalar values, rather than using the number of + // extended grapheme clusters (a.k.a. "characters" in the end user's mind). void _handleKeyEvent(RawKeyEvent keyEvent) { - // Only handle key events on Android. - if (keyEvent.data is! RawKeyEventDataAndroid) + if (keyEvent is! RawKeyDownEvent || onSelectionChanged == null) return; - if (keyEvent is RawKeyUpEvent) + final Set keysPressed = LogicalKeyboardKey.collapseSynonyms(RawKeyboard.instance.keysPressed); + final LogicalKeyboardKey key = keyEvent.logicalKey; + + if (!_nonModifierKeys.contains(key) || + keysPressed.difference(_modifierKeys).length > 1 || + keysPressed.difference(_interestingKeys).isNotEmpty) { + // If the most recently pressed key isn't a non-modifier key, or more than + // one non-modifier key is down, or keys other than the ones we're interested in + // are pressed, just ignore the keypress. return; - - final RawKeyEventDataAndroid rawAndroidEvent = keyEvent.data; - final int pressedKeyCode = rawAndroidEvent.keyCode; - final int pressedKeyMetaState = rawAndroidEvent.metaState; - - if (selection.isCollapsed) { - _extentOffset = selection.extentOffset; - _baseOffset = selection.baseOffset; } - // Update current key states - final bool shift = pressedKeyMetaState & _kShiftMask > 0; - final bool ctrl = pressedKeyMetaState & _kControlMask > 0; - - final bool rightArrow = pressedKeyCode == _kRightArrowCode; - final bool leftArrow = pressedKeyCode == _kLeftArrowCode; - final bool upArrow = pressedKeyCode == _kUpArrowCode; - final bool downArrow = pressedKeyCode == _kDownArrowCode; - final bool arrow = leftArrow || rightArrow || upArrow || downArrow; - final bool aKey = pressedKeyCode == _kAKeyCode; - final bool xKey = pressedKeyCode == _kXKeyCode; - final bool vKey = pressedKeyCode == _kVKeyCode; - final bool cKey = pressedKeyCode == _kCKeyCode; - final bool del = pressedKeyCode == _kDelKeyCode; - - // We will only move select or more the caret if an arrow is pressed - if (arrow) { - int newOffset = _extentOffset; - - // Because the user can use multiple keys to change how he selects - // the new offset variable is threaded through these four functions - // and potentially changes after each one. - if (ctrl) - newOffset = _handleControl(rightArrow, leftArrow, ctrl, newOffset); - newOffset = _handleHorizontalArrows(rightArrow, leftArrow, shift, newOffset); - if (downArrow || upArrow) - newOffset = _handleVerticalArrows(upArrow, downArrow, shift, newOffset); - newOffset = _handleShift(rightArrow, leftArrow, shift, newOffset); - - _extentOffset = newOffset; - } else if (ctrl && (xKey || vKey || cKey || aKey)) { - // _handleShortcuts depends on being started in the same stack invocation as the _handleKeyEvent method - _handleShortcuts(pressedKeyCode); - } - if (del) + if (_movementKeys.contains(key)) { + _handleMovement(key, control: keyEvent.isControlPressed, shift: keyEvent.isShiftPressed); + } else if (keyEvent.isControlPressed && _shortcutKeys.contains(key)) { + // _handleShortcuts depends on being started in the same stack invocation + // as the _handleKeyEvent method + _handleShortcuts(key); + } else if (key == LogicalKeyboardKey.delete) { _handleDelete(); - } - - // Handles full word traversal using control. - int _handleControl(bool rightArrow, bool leftArrow, bool ctrl, int newOffset) { - // If control is pressed, we will decide which way to look for a word - // based on which arrow is pressed. - if (leftArrow && _extentOffset > 2) { - final TextSelection textSelection = _selectWordAtOffset(TextPosition(offset: _extentOffset - 2)); - newOffset = textSelection.baseOffset + 1; - } else if (rightArrow && _extentOffset < text.toPlainText().length - 2) { - final TextSelection textSelection = _selectWordAtOffset(TextPosition(offset: _extentOffset + 1)); - newOffset = textSelection.extentOffset - 1; } - return newOffset; } - int _handleHorizontalArrows(bool rightArrow, bool leftArrow, bool shift, int newOffset) { + void _handleMovement( + LogicalKeyboardKey key, { + @required bool control, + @required bool shift, + }) { + TextSelection newSelection = selection; + + final bool rightArrow = key == LogicalKeyboardKey.arrowRight; + final bool leftArrow = key == LogicalKeyboardKey.arrowLeft; + final bool upArrow = key == LogicalKeyboardKey.arrowUp; + final bool downArrow = key == LogicalKeyboardKey.arrowDown; + + // Because the user can use multiple keys to change how they select, the + // new offset variable is threaded through these four functions and + // potentially changes after each one. + if (control) { + // If control is pressed, we will decide which way to look for a word + // based on which arrow is pressed. + if (leftArrow && newSelection.extentOffset > 2) { + final TextSelection textSelection = _selectWordAtOffset(TextPosition(offset: newSelection.extentOffset - 2)); + newSelection = newSelection.copyWith(extentOffset: textSelection.baseOffset + 1); + } else if (rightArrow && newSelection.extentOffset < text.toPlainText().length - 2) { + final TextSelection textSelection = _selectWordAtOffset(TextPosition(offset: newSelection.extentOffset + 1)); + newSelection = newSelection.copyWith(extentOffset: textSelection.extentOffset - 1); + } + } // Set the new offset to be +/- 1 depending on which arrow is pressed // If shift is down, we also want to update the previous cursor location - if (rightArrow && _extentOffset < text.toPlainText().length) { - newOffset += 1; - if (shift) - _previousCursorLocation += 1; - } - if (leftArrow && _extentOffset > 0) { - newOffset -= 1; - if (shift) - _previousCursorLocation -= 1; - } - return newOffset; - } - - // Handles moving the cursor vertically as well as taking care of the - // case where the user moves the cursor to the end or beginning of the text - // and then back up or down. - int _handleVerticalArrows(bool upArrow, bool downArrow, bool shift, int newOffset) { - // The caret offset gives a location in the upper left hand corner of - // the caret so the middle of the line above is a half line above that - // point and the line below is 1.5 lines below that point. - final double plh = _textPainter.preferredLineHeight; - final double verticalOffset = upArrow ? -0.5 * plh : 1.5 * plh; - - final Offset caretOffset = _textPainter.getOffsetForCaret(TextPosition(offset: _extentOffset), _caretPrototype); - final Offset caretOffsetTranslated = caretOffset.translate(0.0, verticalOffset); - final TextPosition position = _textPainter.getPositionForOffset(caretOffsetTranslated); - - // To account for the possibility where the user vertically highlights - // all the way to the top or bottom of the text, we hold the previous - // cursor location. This allows us to restore to this position in the - // case that the user wants to unhighlight some text. - if (position.offset == _extentOffset) { - if (downArrow) - newOffset = text.toPlainText().length; - else if (upArrow) - newOffset = 0; - _resetCursor = shift; - } else if (_resetCursor && shift) { - newOffset = _previousCursorLocation; - _resetCursor = false; - } else { - newOffset = position.offset; - _previousCursorLocation = newOffset; - } - return newOffset; - } - - // Handles the selection of text or removal of the selection and placing - // of the caret. - int _handleShift(bool rightArrow, bool leftArrow, bool shift, int newOffset) { - if (onSelectionChanged == null) - return newOffset; - // In the text_selection class, a TextSelection is defined such that the - // base offset is always less than the extent offset. - if (shift) { - if (_baseOffset < newOffset) { - _handlePotentialSelectionChange( - TextSelection( - baseOffset: _baseOffset, - extentOffset: newOffset, - ), - SelectionChangedCause.keyboard, - ); - } else { - _handlePotentialSelectionChange( - TextSelection( - baseOffset: newOffset, - extentOffset: _baseOffset, - ), - SelectionChangedCause.keyboard, - ); + if (rightArrow && newSelection.extentOffset < text.toPlainText().length) { + newSelection = newSelection.copyWith(extentOffset: newSelection.extentOffset + 1); + if (shift) { + _cursorResetLocation += 1; } - } else { + } + if (leftArrow && newSelection.extentOffset > 0) { + newSelection = newSelection.copyWith(extentOffset: newSelection.extentOffset - 1); + if (shift) { + _cursorResetLocation -= 1; + } + } + // Handles moving the cursor vertically as well as taking care of the + // case where the user moves the cursor to the end or beginning of the text + // and then back up or down. + if (downArrow || upArrow) { + // The caret offset gives a location in the upper left hand corner of + // the caret so the middle of the line above is a half line above that + // point and the line below is 1.5 lines below that point. + final double preferredLineHeight = _textPainter.preferredLineHeight; + final double verticalOffset = upArrow ? -0.5 * preferredLineHeight : 1.5 * preferredLineHeight; + + final Offset caretOffset = _textPainter.getOffsetForCaret(TextPosition(offset: newSelection.extentOffset), _caretPrototype); + final Offset caretOffsetTranslated = caretOffset.translate(0.0, verticalOffset); + final TextPosition position = _textPainter.getPositionForOffset(caretOffsetTranslated); + + // To account for the possibility where the user vertically highlights + // all the way to the top or bottom of the text, we hold the previous + // cursor location. This allows us to restore to this position in the + // case that the user wants to unhighlight some text. + if (position.offset == newSelection.extentOffset) { + if (downArrow) { + newSelection = newSelection.copyWith(extentOffset: text.toPlainText().length); + } else if (upArrow) { + newSelection = newSelection.copyWith(extentOffset: 0); + } + _wasSelectingVerticallyWithKeyboard = shift; + } else if (_wasSelectingVerticallyWithKeyboard && shift) { + newSelection = newSelection.copyWith(extentOffset: _cursorResetLocation); + _wasSelectingVerticallyWithKeyboard = false; + } else { + newSelection = newSelection.copyWith(extentOffset: position.offset); + _cursorResetLocation = newSelection.extentOffset; + } + } + + // Just place the collapsed selection at the new position if shift isn't down. + if (!shift) { // We want to put the cursor at the correct location depending on which // arrow is used while there is a selection. + int newOffset = newSelection.extentOffset; if (!selection.isCollapsed) { if (leftArrow) - newOffset = _baseOffset < _extentOffset ? _baseOffset : _extentOffset; + newOffset = newSelection.baseOffset < newSelection.extentOffset ? newSelection.baseOffset : newSelection.extentOffset; else if (rightArrow) - newOffset = _baseOffset > _extentOffset ? _baseOffset : _extentOffset; + newOffset = newSelection.baseOffset > newSelection.extentOffset ? newSelection.baseOffset : newSelection.extentOffset; } - _handlePotentialSelectionChange( - TextSelection.fromPosition( - TextPosition( - offset: newOffset - ) - ), - SelectionChangedCause.keyboard, - ); + newSelection = TextSelection.fromPosition(TextPosition(offset: newOffset)); } - return newOffset; + + _handleSelectionChange( + newSelection, + SelectionChangedCause.keyboard, + ); } // Handles shortcut functionality including cut, copy, paste and select all // using control + (X, C, V, A). - Future _handleShortcuts(int pressedKeyCode) async { - switch (pressedKeyCode) { - case _kCKeyCode: - if (!selection.isCollapsed) { - Clipboard.setData( + Future _handleShortcuts(LogicalKeyboardKey key) async { + assert(_shortcutKeys.contains(key), 'shortcut key $key not recognized.'); + if (key == LogicalKeyboardKey.keyC) { + if (!selection.isCollapsed) { + Clipboard.setData( ClipboardData(text: selection.textInside(text.toPlainText()))); - } - break; - case _kXKeyCode: - if (!selection.isCollapsed) { - Clipboard.setData( - ClipboardData(text: selection.textInside(text.toPlainText()))); - textSelectionDelegate.textEditingValue = TextEditingValue( - text: selection.textBefore(text.toPlainText()) + } + return; + } + if (key == LogicalKeyboardKey.keyX) { + if (!selection.isCollapsed) { + Clipboard.setData(ClipboardData(text: selection.textInside(text.toPlainText()))); + textSelectionDelegate.textEditingValue = TextEditingValue( + text: selection.textBefore(text.toPlainText()) + selection.textAfter(text.toPlainText()), - selection: TextSelection.collapsed(offset: selection.start), - ); - } - break; - case _kVKeyCode: - // Snapshot the input before using `await`. - // See https://github.com/flutter/flutter/issues/11427 - final TextEditingValue value = textSelectionDelegate.textEditingValue; - final ClipboardData data = await Clipboard.getData(Clipboard.kTextPlain); - if (data != null) { - textSelectionDelegate.textEditingValue = TextEditingValue( - text: value.selection.textBefore(value.text) + selection: TextSelection.collapsed(offset: selection.start), + ); + } + return; + } + if (key == LogicalKeyboardKey.keyV) { + // Snapshot the input before using `await`. + // See https://github.com/flutter/flutter/issues/11427 + final TextEditingValue value = textSelectionDelegate.textEditingValue; + final ClipboardData data = await Clipboard.getData(Clipboard.kTextPlain); + if (data != null) { + textSelectionDelegate.textEditingValue = TextEditingValue( + text: value.selection.textBefore(value.text) + data.text + value.selection.textAfter(value.text), - selection: TextSelection.collapsed( + selection: TextSelection.collapsed( offset: value.selection.start + data.text.length - ), - ); - } - break; - case _kAKeyCode: - _baseOffset = 0; - _extentOffset = textSelectionDelegate.textEditingValue.text.length; - _handlePotentialSelectionChange( - TextSelection( - baseOffset: 0, - extentOffset: textSelectionDelegate.textEditingValue.text.length, ), - SelectionChangedCause.keyboard, ); - break; - default: - assert(false); + } + return; + } + if (key == LogicalKeyboardKey.keyA) { + _handleSelectionChange( + selection.copyWith( + baseOffset: 0, + extentOffset: textSelectionDelegate.textEditingValue.text.length, + ), + SelectionChangedCause.keyboard, + ); + return; } } @@ -1066,7 +1046,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin { } void _handleSetSelection(TextSelection selection) { - _handlePotentialSelectionChange(selection, SelectionChangedCause.keyboard); + _handleSelectionChange(selection, SelectionChangedCause.keyboard); } void _handleMoveCursorForwardByCharacter(bool extentSelection) { @@ -1074,7 +1054,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin { if (extentOffset == null) return; final int baseOffset = !extentSelection ? extentOffset : _selection.baseOffset; - _handlePotentialSelectionChange( + _handleSelectionChange( TextSelection(baseOffset: baseOffset, extentOffset: extentOffset), SelectionChangedCause.keyboard, ); } @@ -1084,7 +1064,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin { if (extentOffset == null) return; final int baseOffset = !extentSelection ? extentOffset : _selection.baseOffset; - _handlePotentialSelectionChange( + _handleSelectionChange( TextSelection(baseOffset: baseOffset, extentOffset: extentOffset), SelectionChangedCause.keyboard, ); } @@ -1097,7 +1077,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin { if (nextWord == null) return; final int baseOffset = extentSelection ? _selection.baseOffset : nextWord.start; - _handlePotentialSelectionChange( + _handleSelectionChange( TextSelection( baseOffset: baseOffset, extentOffset: nextWord.start, @@ -1114,7 +1094,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin { if (previousWord == null) return; final int baseOffset = extentSelection ? _selection.baseOffset : previousWord.start; - _handlePotentialSelectionChange( + _handleSelectionChange( TextSelection( baseOffset: baseOffset, extentOffset: previousWord.start, @@ -1475,29 +1455,28 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin { assert(cause != null); assert(from != null); _layoutText(minWidth: constraints.minWidth, maxWidth: constraints.maxWidth); - if (onSelectionChanged != null) { - final TextPosition fromPosition = _textPainter.getPositionForOffset(globalToLocal(from - _paintOffset)); - final TextPosition toPosition = to == null - ? null - : _textPainter.getPositionForOffset(globalToLocal(to - _paintOffset)); - - int baseOffset = fromPosition.offset; - int extentOffset = fromPosition.offset; - if (toPosition != null) { - baseOffset = math.min(fromPosition.offset, toPosition.offset); - extentOffset = math.max(fromPosition.offset, toPosition.offset); - } - - final TextSelection newSelection = TextSelection( - baseOffset: baseOffset, - extentOffset: extentOffset, - affinity: fromPosition.affinity, - ); - // Call [onSelectionChanged] only when the selection actually changed. - if (newSelection != _selection) { - _handlePotentialSelectionChange(newSelection, cause); - } + if (onSelectionChanged == null) { + return; } + final TextPosition fromPosition = _textPainter.getPositionForOffset(globalToLocal(from - _paintOffset)); + final TextPosition toPosition = to == null + ? null + : _textPainter.getPositionForOffset(globalToLocal(to - _paintOffset)); + + int baseOffset = fromPosition.offset; + int extentOffset = fromPosition.offset; + if (toPosition != null) { + baseOffset = math.min(fromPosition.offset, toPosition.offset); + extentOffset = math.max(fromPosition.offset, toPosition.offset); + } + + final TextSelection newSelection = TextSelection( + baseOffset: baseOffset, + extentOffset: extentOffset, + affinity: fromPosition.affinity, + ); + // Call [onSelectionChanged] only when the selection actually changed. + _handleSelectionChange(newSelection, cause); } /// Select a word around the location of the last tap down. @@ -1517,21 +1496,22 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin { assert(cause != null); assert(from != null); _layoutText(minWidth: constraints.minWidth, maxWidth: constraints.maxWidth); - if (onSelectionChanged != null) { - final TextPosition firstPosition = _textPainter.getPositionForOffset(globalToLocal(from - _paintOffset)); - final TextSelection firstWord = _selectWordAtOffset(firstPosition); - final TextSelection lastWord = to == null ? - firstWord : _selectWordAtOffset(_textPainter.getPositionForOffset(globalToLocal(to - _paintOffset))); - - _handlePotentialSelectionChange( - TextSelection( - baseOffset: firstWord.base.offset, - extentOffset: lastWord.extent.offset, - affinity: firstWord.affinity, - ), - cause, - ); + if (onSelectionChanged == null) { + return; } + final TextPosition firstPosition = _textPainter.getPositionForOffset(globalToLocal(from - _paintOffset)); + final TextSelection firstWord = _selectWordAtOffset(firstPosition); + final TextSelection lastWord = to == null ? + firstWord : _selectWordAtOffset(_textPainter.getPositionForOffset(globalToLocal(to - _paintOffset))); + + _handleSelectionChange( + TextSelection( + baseOffset: firstWord.base.offset, + extentOffset: lastWord.extent.offset, + affinity: firstWord.affinity, + ), + cause, + ); } /// Move the selection to the beginning or end of a word. @@ -1541,20 +1521,21 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin { assert(cause != null); _layoutText(minWidth: constraints.minWidth, maxWidth: constraints.maxWidth); assert(_lastTapDownPosition != null); - if (onSelectionChanged != null) { - final TextPosition position = _textPainter.getPositionForOffset(globalToLocal(_lastTapDownPosition - _paintOffset)); - final TextRange word = _textPainter.getWordBoundary(position); - if (position.offset - word.start <= 1) { - _handlePotentialSelectionChange( - TextSelection.collapsed(offset: word.start, affinity: TextAffinity.downstream), - cause, - ); - } else { - _handlePotentialSelectionChange( - TextSelection.collapsed(offset: word.end, affinity: TextAffinity.upstream), - cause, - ); - } + if (onSelectionChanged == null) { + return; + } + final TextPosition position = _textPainter.getPositionForOffset(globalToLocal(_lastTapDownPosition - _paintOffset)); + final TextRange word = _textPainter.getWordBoundary(position); + if (position.offset - word.start <= 1) { + _handleSelectionChange( + TextSelection.collapsed(offset: word.start, affinity: TextAffinity.downstream), + cause, + ); + } else { + _handleSelectionChange( + TextSelection.collapsed(offset: word.end, affinity: TextAffinity.upstream), + cause, + ); } } diff --git a/packages/flutter/lib/src/services/keyboard_key.dart b/packages/flutter/lib/src/services/keyboard_key.dart index 57465b3a5f..3bf138b0a6 100644 --- a/packages/flutter/lib/src/services/keyboard_key.dart +++ b/packages/flutter/lib/src/services/keyboard_key.dart @@ -238,6 +238,21 @@ class LogicalKeyboardKey extends KeyboardKey { return result == null ? {} : {result}; } + /// Takes a set of keys, and returns the same set, but with any keys that have + /// synonyms replaced. + /// + /// It is used, for example, to make sets of keys with members like + /// [controlRight] and [controlLeft] and convert that set to contain just + /// [control], so that the question "is any control key down?" can be asked. + static Set collapseSynonyms(Set input) { + final Set result = {}; + for (LogicalKeyboardKey key in input) { + final LogicalKeyboardKey synonym = _synonyms[key]; + result.add(synonym ?? key); + } + return result; + } + @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index 123dcf3159..7ef57245cd 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -3655,8 +3655,9 @@ void main() { await tester.pumpAndSettle(); await tester.sendKeyDownEvent(LogicalKeyboardKey.shift); - await tester.sendKeyDownEvent(LogicalKeyboardKey.arrowLeft); - expect(controller.selection.extentOffset - controller.selection.baseOffset, 1); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft); + await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); + expect(controller.selection.extentOffset - controller.selection.baseOffset, -1); }); testWidgets('Shift test 2', (WidgetTester tester) async { @@ -3709,7 +3710,7 @@ void main() { await tester.sendKeyDownEvent(LogicalKeyboardKey.arrowUp); await tester.pumpAndSettle(); - expect(controller.selection.extentOffset - controller.selection.baseOffset, 11); + expect(controller.selection.extentOffset - controller.selection.baseOffset, -11); await tester.sendKeyUpEvent(LogicalKeyboardKey.arrowUp); await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); @@ -3774,7 +3775,7 @@ void main() { await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); await tester.pumpAndSettle(); - expect(controller.selection.extentOffset - controller.selection.baseOffset, 5); + expect(controller.selection.extentOffset - controller.selection.baseOffset, -5); }); testWidgets('Read only keyboard selection test', (WidgetTester tester) async { @@ -3794,7 +3795,7 @@ void main() { await tester.sendKeyDownEvent(LogicalKeyboardKey.shift); await tester.sendKeyDownEvent(LogicalKeyboardKey.arrowLeft); - expect(controller.selection.extentOffset - controller.selection.baseOffset, 1); + expect(controller.selection.extentOffset - controller.selection.baseOffset, -1); }); }); @@ -3867,8 +3868,8 @@ void main() { await tester.sendKeyUpEvent(LogicalKeyboardKey.controlRight); await tester.pumpAndSettle(); - const String expected = 'a biga big house\njumped over a mouse'; - expect(find.text(expected), findsOneWidget); + const String expected = 'a big a bighouse\njumped over a mouse'; + expect(find.text(expected), findsOneWidget, reason: 'Because text contains ${controller.text}'); }); testWidgets('Cut test', (WidgetTester tester) async { @@ -4100,7 +4101,7 @@ void main() { } await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); - expect(c1.selection.extentOffset - c1.selection.baseOffset, 5); + expect(c1.selection.extentOffset - c1.selection.baseOffset, -5); await tester.pumpWidget( MaterialApp( @@ -4136,7 +4137,7 @@ void main() { } await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); - expect(c1.selection.extentOffset - c1.selection.baseOffset, 10); + expect(c1.selection.extentOffset - c1.selection.baseOffset, -10); }); @@ -4193,7 +4194,7 @@ void main() { } await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); - expect(c1.selection.extentOffset - c1.selection.baseOffset, 5); + expect(c1.selection.extentOffset - c1.selection.baseOffset, -5); expect(c2.selection.extentOffset - c2.selection.baseOffset, 0); await tester.enterText(find.byType(TextField).last, testValue); @@ -4212,7 +4213,7 @@ void main() { await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); expect(c1.selection.extentOffset - c1.selection.baseOffset, 0); - expect(c2.selection.extentOffset - c2.selection.baseOffset, 5); + expect(c2.selection.extentOffset - c2.selection.baseOffset, -5); }); testWidgets('Caret works when maxLines is null', (WidgetTester tester) async { diff --git a/packages/flutter/test/rendering/editable_test.dart b/packages/flutter/test/rendering/editable_test.dart index c01d24eed7..c4ceb609bb 100644 --- a/packages/flutter/test/rendering/editable_test.dart +++ b/packages/flutter/test/rendering/editable_test.dart @@ -12,7 +12,7 @@ import '../rendering/mock_canvas.dart'; import '../rendering/recording_canvas.dart'; import 'rendering_tester.dart'; -class FakeEditableTextState extends TextSelectionDelegate { +class FakeEditableTextState with TextSelectionDelegate { @override TextEditingValue get textEditingValue { return const TextEditingValue(); } diff --git a/packages/flutter/test/services/keyboard_key_test.dart b/packages/flutter/test/services/keyboard_key_test.dart index ea60e939d7..1e6d3e2525 100644 --- a/packages/flutter/test/services/keyboard_key_test.dart +++ b/packages/flutter/test/services/keyboard_key_test.dart @@ -60,5 +60,51 @@ void main() { expect(LogicalKeyboardKey.altRight.synonyms.first, equals(LogicalKeyboardKey.alt)); expect(LogicalKeyboardKey.metaRight.synonyms.first, equals(LogicalKeyboardKey.meta)); }); + test('Synonyms get collapsed properly.', () async { + expect(LogicalKeyboardKey.collapseSynonyms({}), isEmpty); + expect( + LogicalKeyboardKey.collapseSynonyms({ + LogicalKeyboardKey.shiftLeft, + LogicalKeyboardKey.controlLeft, + LogicalKeyboardKey.altLeft, + LogicalKeyboardKey.metaLeft, + }), + equals({ + LogicalKeyboardKey.shift, + LogicalKeyboardKey.control, + LogicalKeyboardKey.alt, + LogicalKeyboardKey.meta, + })); + expect( + LogicalKeyboardKey.collapseSynonyms({ + LogicalKeyboardKey.shiftRight, + LogicalKeyboardKey.controlRight, + LogicalKeyboardKey.altRight, + LogicalKeyboardKey.metaRight, + }), + equals({ + LogicalKeyboardKey.shift, + LogicalKeyboardKey.control, + LogicalKeyboardKey.alt, + LogicalKeyboardKey.meta, + })); + expect( + LogicalKeyboardKey.collapseSynonyms({ + LogicalKeyboardKey.shiftLeft, + LogicalKeyboardKey.controlLeft, + LogicalKeyboardKey.altLeft, + LogicalKeyboardKey.metaLeft, + LogicalKeyboardKey.shiftRight, + LogicalKeyboardKey.controlRight, + LogicalKeyboardKey.altRight, + LogicalKeyboardKey.metaRight, + }), + equals({ + LogicalKeyboardKey.shift, + LogicalKeyboardKey.control, + LogicalKeyboardKey.alt, + LogicalKeyboardKey.meta, + })); + }); }); } diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index 4ff39a8434..98a006f4d2 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -26,7 +26,27 @@ enum HandlePositionInViewport { leftEdge, rightEdge, within, } +class MockClipboard { + Object _clipboardData = { + 'text': null, + }; + + Future handleMethodCall(MethodCall methodCall) async { + switch (methodCall.method) { + case 'Clipboard.getData': + return _clipboardData; + case 'Clipboard.setData': + _clipboardData = methodCall.arguments; + break; + } + } +} + void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + final MockClipboard mockClipboard = MockClipboard(); + SystemChannels.platform.setMockMethodCallHandler(mockClipboard.handleMethodCall); + setUp(() { debugResetSemanticsIdCounter(); controller = TextEditingController(); @@ -785,12 +805,7 @@ void main() { // Populate a fake clipboard. const String clipboardContent = 'Dobunezumi mitai ni utsukushiku naritai'; - SystemChannels.platform - .setMockMethodCallHandler((MethodCall methodCall) async { - if (methodCall.method == 'Clipboard.getData') - return const {'text': clipboardContent}; - return null; - }); + Clipboard.setData(const ClipboardData(text: clipboardContent)); // Long-press to bring up the text editing controls. final Finder textFinder = find.byType(EditableText); @@ -2760,6 +2775,360 @@ void main() { expect(controller.selection.extent.offset, 5); }, skip: isBrowser); + testWidgets('keyboard text selection works as expected', (WidgetTester tester) async { + // Text with two separate words to select. + const String testText = 'Now is the time for\n' + 'all good people\n' + 'to come to the aid\n' + 'of their country.'; + final TextEditingController controller = TextEditingController(text: testText); + controller.selection = const TextSelection( + baseOffset: 0, + extentOffset: 0, + affinity: TextAffinity.upstream, + ); + TextSelection selection; + SelectionChangedCause cause; + await tester.pumpWidget(MaterialApp( + home: Align( + alignment: Alignment.topLeft, + child: SizedBox( + width: 400, + child: EditableText( + maxLines: 10, + controller: controller, + showSelectionHandles: true, + autofocus: true, + focusNode: FocusNode(), + style: Typography(platform: TargetPlatform.android).black.subhead, + cursorColor: Colors.blue, + backgroundCursorColor: Colors.grey, + selectionControls: materialTextSelectionControls, + keyboardType: TextInputType.text, + textAlign: TextAlign.right, + onSelectionChanged: (TextSelection newSelection, SelectionChangedCause newCause) { + selection = newSelection; + cause = newCause; + }, + ), + ), + ), + )); + + await tester.pump(); // Wait for autofocus to take effect. + + Future sendKeys(List keys, {bool shift = false, bool control = false}) async { + if (shift) { + await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); + } + if (control) { + await tester.sendKeyDownEvent(LogicalKeyboardKey.controlLeft); + } + for (LogicalKeyboardKey key in keys) { + await tester.sendKeyEvent(key); + await tester.pump(); + } + if (control) { + await tester.sendKeyUpEvent(LogicalKeyboardKey.controlLeft); + } + if (shift) { + await tester.sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); + } + if (shift || control) { + await tester.pump(); + } + } + + // Select a few characters using shift right arrow + await sendKeys( + [ + LogicalKeyboardKey.arrowRight, + LogicalKeyboardKey.arrowRight, + LogicalKeyboardKey.arrowRight, + ], + shift: true, + ); + + expect(cause, equals(SelectionChangedCause.keyboard)); + expect(selection, equals(const TextSelection( + baseOffset: 0, + extentOffset: 3, + affinity: TextAffinity.upstream, + ))); + + // Select fewer characters using shift left arrow + await sendKeys( + [ + LogicalKeyboardKey.arrowLeft, + LogicalKeyboardKey.arrowLeft, + LogicalKeyboardKey.arrowLeft, + ], + shift: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 0, + extentOffset: 0, + affinity: TextAffinity.upstream, + ))); + + // Try to select before the first character, nothing should change. + await sendKeys( + [ + LogicalKeyboardKey.arrowLeft, + ], + shift: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 0, + extentOffset: 0, + affinity: TextAffinity.upstream, + ))); + + // Select the first two words. + await sendKeys( + [ + LogicalKeyboardKey.arrowRight, + LogicalKeyboardKey.arrowRight, + ], + shift: true, + control: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 0, + extentOffset: 6, + affinity: TextAffinity.upstream, + ))); + + // Unselect the second word. + await sendKeys( + [ + LogicalKeyboardKey.arrowLeft, + ], + shift: true, + control: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 0, + extentOffset: 4, + affinity: TextAffinity.upstream, + ))); + + // Select the next line. + await sendKeys( + [ + LogicalKeyboardKey.arrowDown, + ], + shift: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 0, + extentOffset: 20, + affinity: TextAffinity.upstream, + ))); + + // Move forward one character to reset the selection. + await sendKeys( + [ + LogicalKeyboardKey.arrowRight, + ], + ); + + expect(selection, equals(const TextSelection( + baseOffset: 21, + extentOffset: 21, + affinity: TextAffinity.downstream, + ))); + + // Select the next line. + await sendKeys( + [ + LogicalKeyboardKey.arrowDown, + ], + shift: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 21, + extentOffset: 40, + affinity: TextAffinity.downstream, + ))); + + // Select to the end of the string by going down. + await sendKeys( + [ + LogicalKeyboardKey.arrowDown, + LogicalKeyboardKey.arrowDown, + LogicalKeyboardKey.arrowDown, + LogicalKeyboardKey.arrowDown, + ], + shift: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 21, + extentOffset: testText.length, + affinity: TextAffinity.downstream, + ))); + + // Go back up one line to set selection up to part of the last line. + await sendKeys( + [ + LogicalKeyboardKey.arrowUp, + ], + shift: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 21, + extentOffset: 58, + affinity: TextAffinity.downstream, + ))); + + // Select All + await sendKeys( + [ + LogicalKeyboardKey.keyA, + ], + control: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 0, + extentOffset: testText.length, + affinity: TextAffinity.downstream, + ))); + + // Jump to beginning of selection. + await sendKeys( + [ + LogicalKeyboardKey.arrowLeft, + ], + ); + + expect(selection, equals(const TextSelection( + baseOffset: 0, + extentOffset: 0, + affinity: TextAffinity.downstream, + ))); + + // Jump forward three words. + await sendKeys( + [ + LogicalKeyboardKey.arrowRight, + LogicalKeyboardKey.arrowRight, + LogicalKeyboardKey.arrowRight, + ], + control: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 10, + extentOffset: 10, + affinity: TextAffinity.downstream, + ))); + + // Select some characters backward. + await sendKeys( + [ + LogicalKeyboardKey.arrowLeft, + LogicalKeyboardKey.arrowLeft, + LogicalKeyboardKey.arrowLeft, + ], + shift: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 10, + extentOffset: 7, + affinity: TextAffinity.downstream, + ))); + + // Select a word backward. + await sendKeys( + [ + LogicalKeyboardKey.arrowLeft, + ], + shift: true, + control: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 10, + extentOffset: 4, + affinity: TextAffinity.downstream, + ))); + expect(controller.text, equals(testText)); + + // Cut + await sendKeys( + [ + LogicalKeyboardKey.keyX, + ], + control: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 10, + extentOffset: 4, + affinity: TextAffinity.downstream, + ))); + expect(controller.text, equals('Now time for\n' + 'all good people\n' + 'to come to the aid\n' + 'of their country.')); + expect((await Clipboard.getData(Clipboard.kTextPlain)).text, equals('is the')); + + // Paste + await sendKeys( + [ + LogicalKeyboardKey.keyV, + ], + control: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 10, + extentOffset: 4, + affinity: TextAffinity.downstream, + ))); + expect(controller.text, equals(testText)); + + // Copy All + await sendKeys( + [ + LogicalKeyboardKey.keyA, + LogicalKeyboardKey.keyC, + ], + control: true, + ); + + expect(selection, equals(const TextSelection( + baseOffset: 0, + extentOffset: testText.length, + affinity: TextAffinity.downstream, + ))); + expect(controller.text, equals(testText)); + expect((await Clipboard.getData(Clipboard.kTextPlain)).text, equals(testText)); + + // Delete + await sendKeys( + [ + LogicalKeyboardKey.delete, + ], + ); + expect(selection, equals(const TextSelection( + baseOffset: 0, + extentOffset: 72, + affinity: TextAffinity.downstream, + ))); + expect(controller.text, isEmpty); + }); + // Regression test for https://github.com/flutter/flutter/issues/31287 testWidgets('iOS text selection handle visibility', (WidgetTester tester) async { debugDefaultTargetPlatformOverride = TargetPlatform.iOS; diff --git a/packages/flutter/test/widgets/selectable_text_test.dart b/packages/flutter/test/widgets/selectable_text_test.dart index e6878830fc..a44c9e3be1 100644 --- a/packages/flutter/test/widgets/selectable_text_test.dart +++ b/packages/flutter/test/widgets/selectable_text_test.dart @@ -1278,7 +1278,7 @@ void main() { await tester.sendKeyDownEvent(LogicalKeyboardKey.shift); await tester.sendKeyDownEvent(LogicalKeyboardKey.arrowLeft); - expect(controller.selection.extentOffset - controller.selection.baseOffset, 1); + expect(controller.selection.extentOffset - controller.selection.baseOffset, -1); }); testWidgets('Shift test 2', (WidgetTester tester) async { @@ -1302,7 +1302,7 @@ void main() { await tester.pumpAndSettle(); - expect(controller.selection.extentOffset - controller.selection.baseOffset, 5); + expect(controller.selection.extentOffset - controller.selection.baseOffset, -5); }); testWidgets('Down and up test', (WidgetTester tester) async { @@ -1312,7 +1312,7 @@ void main() { await tester.sendKeyDownEvent(LogicalKeyboardKey.arrowUp); await tester.pumpAndSettle(); - expect(controller.selection.extentOffset - controller.selection.baseOffset, 11); + expect(controller.selection.extentOffset - controller.selection.baseOffset, -11); await tester.sendKeyUpEvent(LogicalKeyboardKey.arrowUp); await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); @@ -1371,7 +1371,7 @@ void main() { await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); await tester.pumpAndSettle(); - expect(controller.selection.extentOffset - controller.selection.baseOffset, 5); + expect(controller.selection.extentOffset - controller.selection.baseOffset, -5); }); }); @@ -1516,7 +1516,7 @@ void main() { await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); await tester.pumpAndSettle(); - expect(c1.selection.extentOffset - c1.selection.baseOffset, 5); + expect(c1.selection.extentOffset - c1.selection.baseOffset, -5); await tester.pumpWidget( MaterialApp( @@ -1555,7 +1555,7 @@ void main() { editableTextWidget = tester.widget(find.byType(EditableText).last); c1 = editableTextWidget.controller; - expect(c1.selection.extentOffset - c1.selection.baseOffset, 10); + expect(c1.selection.extentOffset - c1.selection.baseOffset, -6); }); @@ -1610,7 +1610,7 @@ void main() { await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); await tester.pumpAndSettle(); - expect(c1.selection.extentOffset - c1.selection.baseOffset, 5); + expect(c1.selection.extentOffset - c1.selection.baseOffset, -5); expect(c2.selection.extentOffset - c2.selection.baseOffset, 0); await tester.tap(find.byType(SelectableText).last); @@ -1625,7 +1625,7 @@ void main() { await tester.pumpAndSettle(); expect(c1.selection.extentOffset - c1.selection.baseOffset, 0); - expect(c2.selection.extentOffset - c2.selection.baseOffset, 5); + expect(c2.selection.extentOffset - c2.selection.baseOffset, -5); }); testWidgets('Caret works when maxLines is null', (WidgetTester tester) async {