Skip to content

Commit e18196d

Browse files
committed
compose: Enforce max topic/content length by code points, following API
Fixes zulip#1238
1 parent 3708cb4 commit e18196d

File tree

2 files changed

+62
-22
lines changed

2 files changed

+62
-22
lines changed

lib/widgets/compose_box.dart

+36-10
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,30 @@ 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+
int? get lengthUnicodeCodePointsIfLong => _lengthUnicodeCodePointsIfLong;
49+
late int? _lengthUnicodeCodePointsIfLong;
50+
int? _computeLengthUnicodeCodePointsIfLong() =>
51+
_textNormalized.length > maxLengthUnicodeCodePoints
52+
? _textNormalized.runes.length
53+
: null;
54+
3555
List<ErrorT> get validationErrors => _validationErrors;
3656
late List<ErrorT> _validationErrors;
3757
List<ErrorT> _computeValidationErrors();
@@ -40,6 +60,8 @@ abstract class ComposeController<ErrorT> extends TextEditingController {
4060

4161
void _update() {
4262
_textNormalized = _computeTextNormalized();
63+
// uses _textNormalized, so comes after _computeTextNormalized()
64+
_lengthUnicodeCodePointsIfLong = _computeLengthUnicodeCodePointsIfLong();
4365
_validationErrors = _computeValidationErrors();
4466
hasValidationErrors.value = _validationErrors.isNotEmpty;
4567
}
@@ -74,6 +96,9 @@ class ComposeTopicController extends ComposeController<TopicValidationError> {
7496
// https://zulip.com/help/require-topics
7597
final mandatory = true;
7698

99+
// TODO(#307) use `max_topic_length` instead of hardcoded limit
100+
@override final maxLengthUnicodeCodePoints = kMaxTopicLengthCodePoints;
101+
77102
@override
78103
String _computeTextNormalized() {
79104
String trimmed = text.trim();
@@ -86,11 +111,10 @@ class ComposeTopicController extends ComposeController<TopicValidationError> {
86111
if (mandatory && textNormalized == kNoTopicTopic)
87112
TopicValidationError.mandatoryButEmpty,
88113

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)
114+
if (
115+
lengthUnicodeCodePointsIfLong != null
116+
&& lengthUnicodeCodePointsIfLong! > maxLengthUnicodeCodePoints
117+
)
94118
TopicValidationError.tooLong,
95119
];
96120
}
@@ -121,6 +145,9 @@ class ComposeContentController extends ComposeController<ContentValidationError>
121145
_update();
122146
}
123147

148+
// TODO(#1237) use `max_message_length` instead of hardcoded limit
149+
@override final maxLengthUnicodeCodePoints = kMaxMessageLengthCodePoints;
150+
124151
int _nextQuoteAndReplyTag = 0;
125152
int _nextUploadTag = 0;
126153

@@ -262,11 +289,10 @@ class ComposeContentController extends ComposeController<ContentValidationError>
262289
if (textNormalized.isEmpty)
263290
ContentValidationError.empty,
264291

265-
// normalized.length is the number of UTF-16 code units, while the server
266-
// API expresses the max in Unicode code points. So this comparison will
267-
// be conservative and may cut the user off shorter than necessary.
268-
// TODO(#1238) stop cutting off shorter than necessary
269-
if (textNormalized.length > kMaxMessageLengthCodePoints)
292+
if (
293+
lengthUnicodeCodePointsIfLong != null
294+
&& lengthUnicodeCodePointsIfLong! > maxLengthUnicodeCodePoints
295+
)
270296
ContentValidationError.tooLong,
271297

272298
if (_quoteAndReplies.isNotEmpty)

test/widgets/compose_box_test.dart

+26-12
Original file line numberDiff line numberDiff line change
@@ -259,13 +259,20 @@ void main() {
259259
doTest('too-long content is rejected',
260260
content: makeStringWithCodePoints(kMaxMessageLengthCodePoints + 1), expectError: true);
261261

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

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

271278
group('topic', () {
@@ -293,13 +300,20 @@ void main() {
293300
doTest('too-long topic is rejected',
294301
topic: makeStringWithCodePoints(kMaxTopicLengthCodePoints + 1), expectError: true);
295302

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

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

0 commit comments

Comments
 (0)