Unverified Commit 16eb4f2c authored by Justin McCandless's avatar Justin McCandless Committed by GitHub

Gracefully handle negative position in getWordAtOffset (#128464)

Should fix an unreproducible crash in text editing and track it with an assertion.
parent 5cef69dd
...@@ -2067,9 +2067,9 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -2067,9 +2067,9 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
void selectWordsInRange({ required Offset from, Offset? to, required SelectionChangedCause cause }) { void selectWordsInRange({ required Offset from, Offset? to, required SelectionChangedCause cause }) {
_computeTextMetricsIfNeeded(); _computeTextMetricsIfNeeded();
final TextPosition fromPosition = _textPainter.getPositionForOffset(globalToLocal(from - _paintOffset)); final TextPosition fromPosition = _textPainter.getPositionForOffset(globalToLocal(from - _paintOffset));
final TextSelection fromWord = _getWordAtOffset(fromPosition); final TextSelection fromWord = getWordAtOffset(fromPosition);
final TextPosition toPosition = to == null ? fromPosition : _textPainter.getPositionForOffset(globalToLocal(to - _paintOffset)); final TextPosition toPosition = to == null ? fromPosition : _textPainter.getPositionForOffset(globalToLocal(to - _paintOffset));
final TextSelection toWord = toPosition == fromPosition ? fromWord : _getWordAtOffset(toPosition); final TextSelection toWord = toPosition == fromPosition ? fromWord : getWordAtOffset(toPosition);
final bool isFromWordBeforeToWord = fromWord.start < toWord.end; final bool isFromWordBeforeToWord = fromWord.start < toWord.end;
_setSelection( _setSelection(
...@@ -2099,7 +2099,10 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -2099,7 +2099,10 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
_setSelection(newSelection, cause); _setSelection(newSelection, cause);
} }
TextSelection _getWordAtOffset(TextPosition position) { /// Returns a [TextSelection] that encompasses the word at the given
/// [TextPosition].
@visibleForTesting
TextSelection getWordAtOffset(TextPosition position) {
debugAssertLayoutUpToDate(); debugAssertLayoutUpToDate();
// When long-pressing past the end of the text, we want a collapsed cursor. // When long-pressing past the end of the text, we want a collapsed cursor.
if (position.offset >= plainText.length) { if (position.offset >= plainText.length) {
...@@ -2120,6 +2123,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -2120,6 +2123,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
case TextAffinity.downstream: case TextAffinity.downstream:
effectiveOffset = position.offset; effectiveOffset = position.offset;
} }
assert(effectiveOffset >= 0);
// On iOS, select the previous word if there is a previous word, or select // On iOS, select the previous word if there is a previous word, or select
// to the end of the next word if there is a next word. Select nothing if // to the end of the next word if there is a next word. Select nothing if
...@@ -2128,8 +2132,8 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -2128,8 +2132,8 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
// If the platform is Android and the text is read only, try to select the // If the platform is Android and the text is read only, try to select the
// previous word if there is one; otherwise, select the single whitespace at // previous word if there is one; otherwise, select the single whitespace at
// the position. // the position.
if (TextLayoutMetrics.isWhitespace(plainText.codeUnitAt(effectiveOffset)) if (effectiveOffset > 0
&& effectiveOffset > 0) { && TextLayoutMetrics.isWhitespace(plainText.codeUnitAt(effectiveOffset))) {
final TextRange? previousWord = _getPreviousWord(word.start); final TextRange? previousWord = _getPreviousWord(word.start);
switch (defaultTargetPlatform) { switch (defaultTargetPlatform) {
case TargetPlatform.iOS: case TargetPlatform.iOS:
......
...@@ -1823,6 +1823,52 @@ void main() { ...@@ -1823,6 +1823,52 @@ void main() {
rrect: expectedRRect rrect: expectedRRect
)); ));
}); });
test('getWordAtOffset with a negative position', () {
const String text = 'abc';
final _FakeEditableTextState delegate = _FakeEditableTextState()
..textEditingValue = const TextEditingValue(text: text);
final ViewportOffset viewportOffset = ViewportOffset.zero();
final RenderEditable editable = RenderEditable(
backgroundCursorColor: Colors.grey,
selectionColor: Colors.black,
textDirection: TextDirection.ltr,
cursorColor: Colors.red,
offset: viewportOffset,
textSelectionDelegate: delegate,
startHandleLayerLink: LayerLink(),
endHandleLayerLink: LayerLink(),
text: const TextSpan(
text: text,
style: TextStyle(height: 1.0, fontSize: 10.0),
),
);
layout(editable, onErrors: expectNoFlutterErrors);
// Cause text metrics to be computed.
editable.computeDistanceToActualBaseline(TextBaseline.alphabetic);
final TextSelection selection;
try {
selection = editable.getWordAtOffset(const TextPosition(
offset: -1,
affinity: TextAffinity.upstream,
));
} catch (error) {
// In debug mode, negative offsets are caught by an assertion.
expect(error, isA<AssertionError>());
return;
}
// Web's Paragraph.getWordBoundary behaves differently for a negative
// position.
if (kIsWeb) {
expect(selection, const TextSelection.collapsed(offset: 0));
} else {
expect(selection, const TextSelection.collapsed(offset: text.length));
}
});
} }
class _TestRenderEditable extends RenderEditable { class _TestRenderEditable extends RenderEditable {
......
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