diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index fa5652b8ad..3fbf6cca3b 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -28,10 +28,30 @@ const double _composeButtonSize = 44; /// /// Subclasses must ensure that [_update] is called in all exposed constructors. abstract class ComposeController extends TextEditingController { + int get maxLengthUnicodeCodePoints; + String get textNormalized => _textNormalized; late String _textNormalized; String _computeTextNormalized(); + /// Length of [textNormalized] in Unicode code points, + /// if it might exceed [maxLengthUnicodeCodePoints], else null. + /// + /// Use this instead of [String.length] + /// to enforce a max length expressed in code points. + /// [String.length] is conservative and may cut the user off too short. + /// + /// Counting code points ([String.runes]) + /// is more expensive than getting the number of UTF-16 code units + /// ([String.length]), so we avoid it when the result definitely won't exceed + /// [maxLengthUnicodeCodePoints]. + int? get lengthUnicodeCodePointsIfLong => _lengthUnicodeCodePointsIfLong; + late int? _lengthUnicodeCodePointsIfLong; + int? _computeLengthUnicodeCodePointsIfLong() => + _textNormalized.length > maxLengthUnicodeCodePoints + ? _textNormalized.runes.length + : null; + List get validationErrors => _validationErrors; late List _validationErrors; List _computeValidationErrors(); @@ -40,6 +60,8 @@ abstract class ComposeController extends TextEditingController { void _update() { _textNormalized = _computeTextNormalized(); + // uses _textNormalized, so comes after _computeTextNormalized() + _lengthUnicodeCodePointsIfLong = _computeLengthUnicodeCodePointsIfLong(); _validationErrors = _computeValidationErrors(); hasValidationErrors.value = _validationErrors.isNotEmpty; } @@ -74,6 +96,9 @@ class ComposeTopicController extends ComposeController { // https://zulip.com/help/require-topics final mandatory = true; + // TODO(#307) use `max_topic_length` instead of hardcoded limit + @override final maxLengthUnicodeCodePoints = kMaxTopicLengthCodePoints; + @override String _computeTextNormalized() { String trimmed = text.trim(); @@ -86,11 +111,10 @@ class ComposeTopicController extends ComposeController { if (mandatory && textNormalized == kNoTopicTopic) TopicValidationError.mandatoryButEmpty, - // textNormalized.length is the number of UTF-16 code units, while the server - // API expresses the max in Unicode code points. So this comparison will - // be conservative and may cut the user off shorter than necessary. - // TODO(#1238) stop cutting off shorter than necessary - if (textNormalized.length > kMaxTopicLengthCodePoints) + if ( + lengthUnicodeCodePointsIfLong != null + && lengthUnicodeCodePointsIfLong! > maxLengthUnicodeCodePoints + ) TopicValidationError.tooLong, ]; } @@ -121,6 +145,9 @@ class ComposeContentController extends ComposeController _update(); } + // TODO(#1237) use `max_message_length` instead of hardcoded limit + @override final maxLengthUnicodeCodePoints = kMaxMessageLengthCodePoints; + int _nextQuoteAndReplyTag = 0; int _nextUploadTag = 0; @@ -262,11 +289,10 @@ class ComposeContentController extends ComposeController if (textNormalized.isEmpty) ContentValidationError.empty, - // normalized.length is the number of UTF-16 code units, while the server - // API expresses the max in Unicode code points. So this comparison will - // be conservative and may cut the user off shorter than necessary. - // TODO(#1238) stop cutting off shorter than necessary - if (textNormalized.length > kMaxMessageLengthCodePoints) + if ( + lengthUnicodeCodePointsIfLong != null + && lengthUnicodeCodePointsIfLong! > maxLengthUnicodeCodePoints + ) ContentValidationError.tooLong, if (_quoteAndReplies.isNotEmpty) diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index da8863ea32..9b2cfe6bc2 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -259,13 +259,20 @@ void main() { doTest('too-long content is rejected', content: makeStringWithCodePoints(kMaxMessageLengthCodePoints + 1), expectError: true); - // TODO(#1238) unskip - // doTest('max-length content not rejected', - // content: makeStringWithCodePoints(kMaxMessageLengthCodePoints), expectError: false); + doTest('max-length content not rejected', + content: makeStringWithCodePoints(kMaxMessageLengthCodePoints), expectError: false); - // TODO(#1238) replace with above commented-out test - doTest('some content not rejected', - content: 'a' * kMaxMessageLengthCodePoints, expectError: false); + testWidgets('code points not counted unnecessarily', (tester) async { + TypingNotifier.debugEnable = false; + addTearDown(TypingNotifier.debugReset); + + final narrow = ChannelNarrow(channel.streamId); + await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await enterTopic(tester, narrow: narrow, topic: 'some topic'); + await enterContent(tester, narrow: narrow, content: 'a' * kMaxMessageLengthCodePoints); + + check(controller!.content.lengthUnicodeCodePointsIfLong).isNull(); + }); }); group('topic', () { @@ -293,13 +300,20 @@ void main() { doTest('too-long topic is rejected', topic: makeStringWithCodePoints(kMaxTopicLengthCodePoints + 1), expectError: true); - // TODO(#1238) unskip - // doTest('max-length topic not rejected', - // topic: makeStringWithCodePoints(kMaxTopicLengthCodePoints), expectError: false); + doTest('max-length topic not rejected', + topic: makeStringWithCodePoints(kMaxTopicLengthCodePoints), expectError: false); - // TODO(#1238) replace with above commented-out test - doTest('some topic not rejected', - topic: 'a' * kMaxTopicLengthCodePoints, expectError: false); + testWidgets('code points not counted unnecessarily', (tester) async { + TypingNotifier.debugEnable = false; + addTearDown(TypingNotifier.debugReset); + + final narrow = ChannelNarrow(channel.streamId); + await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + await enterTopic(tester, narrow: narrow, topic: 'a' * kMaxTopicLengthCodePoints); + await enterContent(tester, narrow: narrow, content: 'some content'); + + check((controller as StreamComposeBoxController).topic.lengthUnicodeCodePointsIfLong).isNull(); + }); }); });