Skip to content

Commit fe080cb

Browse files
committed
compose: Enforce max topic/content length by code points, following API
Fixes #1238
1 parent 2ce23fe commit fe080cb

File tree

2 files changed

+64
-22
lines changed

2 files changed

+64
-22
lines changed

lib/widgets/compose_box.dart

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,31 @@ const double _composeButtonSize = 44;
2828
///
2929
/// Subclasses must ensure that [_update] is called in all exposed constructors.
3030
abstract class ComposeController<ErrorT> extends TextEditingController {
31+
int get maxLengthUnicodeCodePoints;
32+
3133
String get textNormalized => _textNormalized;
3234
late String _textNormalized;
3335
String _computeTextNormalized();
3436

37+
/// Length of [textNormalized] in Unicode code points
38+
/// if it might exceed [maxLengthUnicodeCodePoints], else null.
39+
///
40+
/// Use this instead of [String.length]
41+
/// to enforce a max length expressed in code points.
42+
/// [String.length] is conservative and may cut the user off too short.
43+
///
44+
/// Counting code points ([String.runes])
45+
/// is more expensive than getting the number of UTF-16 code units
46+
/// ([String.length]), so we avoid it when the result definitely won't exceed
47+
/// [maxLengthUnicodeCodePoints].
48+
late int? _lengthUnicodeCodePointsIfLong;
49+
@visibleForTesting
50+
int? get debugLengthUnicodeCodePointsIfLong => _lengthUnicodeCodePointsIfLong;
51+
int? _computeLengthUnicodeCodePointsIfLong() =>
52+
_textNormalized.length > maxLengthUnicodeCodePoints
53+
? _textNormalized.runes.length
54+
: null;
55+
3556
List<ErrorT> get validationErrors => _validationErrors;
3657
late List<ErrorT> _validationErrors;
3758
List<ErrorT> _computeValidationErrors();
@@ -40,6 +61,8 @@ abstract class ComposeController<ErrorT> extends TextEditingController {
4061

4162
void _update() {
4263
_textNormalized = _computeTextNormalized();
64+
// uses _textNormalized, so comes after _computeTextNormalized()
65+
_lengthUnicodeCodePointsIfLong = _computeLengthUnicodeCodePointsIfLong();
4366
_validationErrors = _computeValidationErrors();
4467
hasValidationErrors.value = _validationErrors.isNotEmpty;
4568
}
@@ -74,6 +97,9 @@ class ComposeTopicController extends ComposeController<TopicValidationError> {
7497
// https://zulip.com/help/require-topics
7598
final mandatory = true;
7699

100+
// TODO(#307) use `max_topic_length` instead of hardcoded limit
101+
@override final maxLengthUnicodeCodePoints = kMaxTopicLengthCodePoints;
102+
77103
@override
78104
String _computeTextNormalized() {
79105
String trimmed = text.trim();
@@ -86,11 +112,10 @@ class ComposeTopicController extends ComposeController<TopicValidationError> {
86112
if (mandatory && textNormalized == kNoTopicTopic)
87113
TopicValidationError.mandatoryButEmpty,
88114

89-
// textNormalized.length is the number of UTF-16 code units, while the server
90-
// API expresses the max in Unicode code points. So this comparison will
91-
// be conservative and may cut the user off shorter than necessary.
92-
// TODO(#1238) stop cutting off shorter than necessary
93-
if (textNormalized.length > kMaxTopicLengthCodePoints)
115+
if (
116+
_lengthUnicodeCodePointsIfLong != null
117+
&& _lengthUnicodeCodePointsIfLong! > maxLengthUnicodeCodePoints
118+
)
94119
TopicValidationError.tooLong,
95120
];
96121
}
@@ -125,6 +150,9 @@ class ComposeContentController extends ComposeController<ContentValidationError>
125150
_update();
126151
}
127152

153+
// TODO(#1237) use `max_message_length` instead of hardcoded limit
154+
@override final maxLengthUnicodeCodePoints = kMaxMessageLengthCodePoints;
155+
128156
int _nextQuoteAndReplyTag = 0;
129157
int _nextUploadTag = 0;
130158

@@ -266,11 +294,10 @@ class ComposeContentController extends ComposeController<ContentValidationError>
266294
if (textNormalized.isEmpty)
267295
ContentValidationError.empty,
268296

269-
// normalized.length is the number of UTF-16 code units, while the server
270-
// API expresses the max in Unicode code points. So this comparison will
271-
// be conservative and may cut the user off shorter than necessary.
272-
// TODO(#1238) stop cutting off shorter than necessary
273-
if (textNormalized.length > kMaxMessageLengthCodePoints)
297+
if (
298+
_lengthUnicodeCodePointsIfLong != null
299+
&& _lengthUnicodeCodePointsIfLong! > maxLengthUnicodeCodePoints
300+
)
274301
ContentValidationError.tooLong,
275302

276303
if (_quoteAndReplies.isNotEmpty)

test/widgets/compose_box_test.dart

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -260,13 +260,20 @@ void main() {
260260
doTest('too-long content is rejected',
261261
content: makeStringWithCodePoints(kMaxMessageLengthCodePoints + 1), expectError: true);
262262

263-
// TODO(#1238) unskip
264-
// doTest('max-length content not rejected',
265-
// content: makeStringWithCodePoints(kMaxMessageLengthCodePoints), expectError: false);
263+
doTest('max-length content not rejected',
264+
content: makeStringWithCodePoints(kMaxMessageLengthCodePoints), expectError: false);
266265

267-
// TODO(#1238) replace with above commented-out test
268-
doTest('some content not rejected',
269-
content: 'a' * kMaxMessageLengthCodePoints, expectError: false);
266+
testWidgets('code points not counted unnecessarily', (tester) async {
267+
TypingNotifier.debugEnable = false;
268+
addTearDown(TypingNotifier.debugReset);
269+
270+
final narrow = ChannelNarrow(channel.streamId);
271+
await prepareComposeBox(tester, narrow: narrow, streams: [channel]);
272+
await enterTopic(tester, narrow: narrow, topic: 'some topic');
273+
await enterContent(tester, narrow: narrow, content: 'a' * kMaxMessageLengthCodePoints);
274+
275+
check(controller!.content.debugLengthUnicodeCodePointsIfLong).isNull();
276+
});
270277
});
271278

272279
group('topic', () {
@@ -294,13 +301,21 @@ void main() {
294301
doTest('too-long topic is rejected',
295302
topic: makeStringWithCodePoints(kMaxTopicLengthCodePoints + 1), expectError: true);
296303

297-
// TODO(#1238) unskip
298-
// doTest('max-length topic not rejected',
299-
// topic: makeStringWithCodePoints(kMaxTopicLengthCodePoints), expectError: false);
304+
doTest('max-length topic not rejected',
305+
topic: makeStringWithCodePoints(kMaxTopicLengthCodePoints), expectError: false);
300306

301-
// TODO(#1238) replace with above commented-out test
302-
doTest('some topic not rejected',
303-
topic: 'a' * kMaxTopicLengthCodePoints, expectError: false);
307+
testWidgets('code points not counted unnecessarily', (tester) async {
308+
TypingNotifier.debugEnable = false;
309+
addTearDown(TypingNotifier.debugReset);
310+
311+
final narrow = ChannelNarrow(channel.streamId);
312+
await prepareComposeBox(tester, narrow: narrow, streams: [channel]);
313+
await enterTopic(tester, narrow: narrow, topic: 'a' * kMaxTopicLengthCodePoints);
314+
await enterContent(tester, narrow: narrow, content: 'some content');
315+
316+
check((controller as StreamComposeBoxController)
317+
.topic.debugLengthUnicodeCodePointsIfLong).isNull();
318+
});
304319
});
305320
});
306321

0 commit comments

Comments
 (0)