Skip to content

Commit 4341ed4

Browse files
justinmcjustinmc
authored
Fix the breaking of multi-code-unit characters in obscure mode (#123366)
This PR changes the character boundary behavior of obscured fields to be based on code points instead of code units. So it used to be possible to traverse and delete obscured text inside of code points (and breaking a code point like that would cause a crash): ![output2](https://github.com/flutter/flutter/assets/389558/674c89a4-c47d-4cdc-a402-4cadb5d2f73b) But now moving the cursor and deleting is based on code points: ![output1](https://github.com/flutter/flutter/assets/389558/e46301f7-b5af-48d2-812a-0ad649f1383b) ### Native behavior Native iOS deletes part of the emoji, native Mac deletes the whole emoji. See flutter/flutter#122381 (comment). So it's unclear what the desired behavior should actually be. ### Resources Fixes flutter/flutter#122381 I thought this might not fix the case where a broken emoji is directly pasted into the field, but it seems to work by trying this: ������� CC @LongCatIsLooong
1 parent ffbeb72 commit 4341ed4

File tree

5 files changed

+509
-28
lines changed

5 files changed

+509
-28
lines changed

‎packages/flutter/lib/src/painting/text_painter.dart‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -170,11 +170,11 @@ class WordBoundary extends TextBoundary {
170170
// single code point that represents a supplementary character.
171171
static int _codePointFromSurrogates(int highSurrogate, int lowSurrogate) {
172172
assert(
173-
TextPainter._isHighSurrogate(highSurrogate),
173+
TextPainter.isHighSurrogate(highSurrogate),
174174
'U+${highSurrogate.toRadixString(16).toUpperCase().padLeft(4, "0")}) is not a high surrogate.',
175175
);
176176
assert(
177-
TextPainter._isLowSurrogate(lowSurrogate),
177+
TextPainter.isLowSurrogate(lowSurrogate),
178178
'U+${lowSurrogate.toRadixString(16).toUpperCase().padLeft(4, "0")}) is not a low surrogate.',
179179
);
180180
const int base = 0x010000 - (0xD800 << 10) - 0xDC00;
@@ -991,17 +991,34 @@ class TextPainter {
991991
canvas.drawParagraph(_paragraph!, offset);
992992
}
993993

994-
// Returns true iff the given value is a valid UTF-16 high surrogate. The value
995-
// must be a UTF-16 code unit, meaning it must be in the range 0x0000-0xFFFF.
996-
//
997-
// See also:
998-
// * https://en.wikipedia.org/wiki/UTF-16#Code_points_from_U+010000_to_U+10FFFF
999-
static bool _isHighSurrogate(int value) {
994+
// Returns true if value falls in the valid range of the UTF16 encoding.
995+
static bool _isUTF16(int value) {
996+
return value >= 0x0 && value <= 0xFFFFF;
997+
}
998+
999+
/// Returns true iff the given value is a valid UTF-16 high (first) surrogate.
1000+
/// The value must be a UTF-16 code unit, meaning it must be in the range
1001+
/// 0x0000-0xFFFF.
1002+
///
1003+
/// See also:
1004+
/// * https://en.wikipedia.org/wiki/UTF-16#Code_points_from_U+010000_to_U+10FFFF
1005+
/// * [isLowSurrogate], which checks the same thing for low (second)
1006+
/// surrogates.
1007+
static bool isHighSurrogate(int value) {
1008+
assert(_isUTF16(value));
10001009
return value & 0xFC00 == 0xD800;
10011010
}
10021011

1003-
// Whether the given UTF-16 code unit is a low (second) surrogate.
1004-
static bool _isLowSurrogate(int value) {
1012+
/// Returns true iff the given value is a valid UTF-16 low (second) surrogate.
1013+
/// The value must be a UTF-16 code unit, meaning it must be in the range
1014+
/// 0x0000-0xFFFF.
1015+
///
1016+
/// See also:
1017+
/// * https://en.wikipedia.org/wiki/UTF-16#Code_points_from_U+010000_to_U+10FFFF
1018+
/// * [isHighSurrogate], which checks the same thing for high (first)
1019+
/// surrogates.
1020+
static bool isLowSurrogate(int value) {
1021+
assert(_isUTF16(value));
10051022
return value & 0xFC00 == 0xDC00;
10061023
}
10071024

@@ -1021,7 +1038,7 @@ class TextPainter {
10211038
return null;
10221039
}
10231040
// TODO(goderbauer): doesn't handle extended grapheme clusters with more than one Unicode scalar value (https://github.com/flutter/flutter/issues/13404).
1024-
return _isHighSurrogate(nextCodeUnit) ? offset + 2 : offset + 1;
1041+
return isHighSurrogate(nextCodeUnit) ? offset + 2 : offset + 1;
10251042
}
10261043

10271044
/// Returns the closest offset before `offset` at which the input cursor can
@@ -1032,7 +1049,7 @@ class TextPainter {
10321049
return null;
10331050
}
10341051
// TODO(goderbauer): doesn't handle extended grapheme clusters with more than one Unicode scalar value (https://github.com/flutter/flutter/issues/13404).
1035-
return _isLowSurrogate(prevCodeUnit) ? offset - 2 : offset - 1;
1052+
return isLowSurrogate(prevCodeUnit) ? offset - 2 : offset - 1;
10361053
}
10371054

10381055
// Unicode value for a zero width joiner character.
@@ -1052,7 +1069,7 @@ class TextPainter {
10521069
const int NEWLINE_CODE_UNIT = 10;
10531070

10541071
// Check for multi-code-unit glyphs such as emojis or zero width joiner.
1055-
final bool needsSearch = _isHighSurrogate(prevCodeUnit) || _isLowSurrogate(prevCodeUnit) || _text!.codeUnitAt(offset) == _zwjUtf16 || _isUnicodeDirectionality(prevCodeUnit);
1072+
final bool needsSearch = isHighSurrogate(prevCodeUnit) || isLowSurrogate(prevCodeUnit) || _text!.codeUnitAt(offset) == _zwjUtf16 || _isUnicodeDirectionality(prevCodeUnit);
10561073
int graphemeClusterLength = needsSearch ? 2 : 1;
10571074
List<TextBox> boxes = <TextBox>[];
10581075
while (boxes.isEmpty) {
@@ -1103,7 +1120,7 @@ class TextPainter {
11031120
final int nextCodeUnit = plainText.codeUnitAt(min(offset, plainTextLength - 1));
11041121

11051122
// Check for multi-code-unit glyphs such as emojis or zero width joiner
1106-
final bool needsSearch = _isHighSurrogate(nextCodeUnit) || _isLowSurrogate(nextCodeUnit) || nextCodeUnit == _zwjUtf16 || _isUnicodeDirectionality(nextCodeUnit);
1123+
final bool needsSearch = isHighSurrogate(nextCodeUnit) || isLowSurrogate(nextCodeUnit) || nextCodeUnit == _zwjUtf16 || _isUnicodeDirectionality(nextCodeUnit);
11071124
int graphemeClusterLength = needsSearch ? 2 : 1;
11081125
List<TextBox> boxes = <TextBox>[];
11091126
while (boxes.isEmpty) {

‎packages/flutter/lib/src/services/text_boundary.dart‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ abstract class TextBoundary {
3131
/// `position`, or null if no boundaries can be found.
3232
///
3333
/// The return value, if not null, is usually less than or equal to `position`.
34+
///
35+
/// The range of the return value is given by the closed interval
36+
/// `[0, string.length]`.
3437
int? getLeadingTextBoundaryAt(int position) {
3538
if (position < 0) {
3639
return null;
@@ -39,10 +42,13 @@ abstract class TextBoundary {
3942
return start >= 0 ? start : null;
4043
}
4144

42-
/// Returns the offset of the closest text boundaries after the given `position`,
43-
/// or null if there is no boundaries can be found after `position`.
45+
/// Returns the offset of the closest text boundary after the given
46+
/// `position`, or null if there is no boundary can be found after `position`.
4447
///
4548
/// The return value, if not null, is usually greater than `position`.
49+
///
50+
/// The range of the return value is given by the closed interval
51+
/// `[0, string.length]`.
4652
int? getTrailingTextBoundaryAt(int position) {
4753
final int end = getTextBoundaryAt(max(0, position)).end;
4854
return end >= 0 ? end : null;

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

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4246,7 +4246,7 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
42464246

42474247
// --------------------------- Text Editing Actions ---------------------------
42484248

4249-
TextBoundary _characterBoundary() => widget.obscureText ? _CodeUnitBoundary(_value.text) : CharacterBoundary(_value.text);
4249+
TextBoundary _characterBoundary() => widget.obscureText ? _CodePointBoundary(_value.text) : CharacterBoundary(_value.text);
42504250
TextBoundary _nextWordBoundary() => widget.obscureText ? _documentBoundary() : renderEditable.wordBoundaries.moveByWordBoundary;
42514251
TextBoundary _linebreak() => widget.obscureText ? _documentBoundary() : LineBoundary(renderEditable);
42524252
TextBoundary _paragraphBoundary() => ParagraphBoundary(_value.text);
@@ -5076,21 +5076,76 @@ class _ScribblePlaceholder extends WidgetSpan {
50765076
}
50775077
}
50785078

5079-
/// A text boundary that uses code units as logical boundaries.
5079+
/// A text boundary that uses code points as logical boundaries.
50805080
///
5081-
/// This text boundary treats every character in input string as an utf-16 code
5082-
/// unit. This can be useful when handling text without any grapheme cluster,
5083-
/// e.g. password input in [EditableText]. If you are handling text that may
5084-
/// include grapheme clusters, consider using [CharacterBoundary].
5085-
class _CodeUnitBoundary extends TextBoundary {
5086-
const _CodeUnitBoundary(this._text);
5081+
/// A code point represents a single character. This may be smaller than what is
5082+
/// represented by a user-perceived character, or grapheme. For example, a
5083+
/// single grapheme (in this case a Unicode extended grapheme cluster) like
5084+
/// "👨‍👩‍👦" consists of five code points: the man emoji, a zero
5085+
/// width joiner, the woman emoji, another zero width joiner, and the boy emoji.
5086+
/// The [String] has a length of eight because each emoji consists of two code
5087+
/// units.
5088+
///
5089+
/// Code units are the units by which Dart's String class is measured, which is
5090+
/// encoded in UTF-16.
5091+
///
5092+
/// See also:
5093+
///
5094+
/// * [String.runes], which deals with code points like this class.
5095+
/// * [String.characters], which deals with graphemes.
5096+
/// * [CharacterBoundary], which is a [TextBoundary] like this class, but whose
5097+
/// boundaries are graphemes instead of code points.
5098+
class _CodePointBoundary extends TextBoundary {
5099+
const _CodePointBoundary(this._text);
50875100

50885101
final String _text;
50895102

5103+
// Returns true if the given position falls in the center of a surrogate pair.
5104+
bool _breaksSurrogatePair(int position) {
5105+
assert(position > 0 && position < _text.length && _text.length > 1);
5106+
return TextPainter.isHighSurrogate(_text.codeUnitAt(position - 1))
5107+
&& TextPainter.isLowSurrogate(_text.codeUnitAt(position));
5108+
}
5109+
50905110
@override
5091-
int getLeadingTextBoundaryAt(int position) => position.clamp(0, _text.length); // ignore_clamp_double_lint
5111+
int? getLeadingTextBoundaryAt(int position) {
5112+
if (_text.isEmpty || position < 0) {
5113+
return null;
5114+
}
5115+
if (position == 0) {
5116+
return 0;
5117+
}
5118+
if (position >= _text.length) {
5119+
return _text.length;
5120+
}
5121+
if (_text.length <= 1) {
5122+
return position;
5123+
}
5124+
5125+
return _breaksSurrogatePair(position)
5126+
? position - 1
5127+
: position;
5128+
}
5129+
50925130
@override
5093-
int getTrailingTextBoundaryAt(int position) => (position + 1).clamp(0, _text.length); // ignore_clamp_double_lint
5131+
int? getTrailingTextBoundaryAt(int position) {
5132+
if (_text.isEmpty || position >= _text.length) {
5133+
return null;
5134+
}
5135+
if (position < 0) {
5136+
return 0;
5137+
}
5138+
if (position == _text.length - 1) {
5139+
return _text.length;
5140+
}
5141+
if (_text.length <= 1) {
5142+
return position;
5143+
}
5144+
5145+
return _breaksSurrogatePair(position + 1)
5146+
? position + 2
5147+
: position + 1;
5148+
}
50945149
}
50955150

50965151
// ------------------------------- Text Actions -------------------------------

‎packages/flutter/test/widgets/editable_text_shortcuts_test.dart‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -444,14 +444,15 @@ void main() {
444444
await tester.pumpWidget(buildEditableText(obscured: true));
445445
await sendKeyCombination(tester, const SingleActivator(trigger));
446446

447+
// Both emojis that were partially selected are deleted entirely.
447448
expect(
448449
controller.text,
449-
'👨‍👩‍👦👨‍👩‍👦',
450+
'‍👩‍👦👨‍👩‍👦',
450451
);
451452

452453
expect(
453454
controller.selection,
454-
const TextSelection.collapsed(offset: 1),
455+
const TextSelection.collapsed(offset: 0),
455456
);
456457
}, variant: TargetPlatformVariant.all(excluding: <TargetPlatform>{ TargetPlatform.iOS }));
457458
});

0 commit comments

Comments
 (0)