diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 0ee11bf169..d279c404ad 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -28,10 +28,31 @@ 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]. + late int? _lengthUnicodeCodePointsIfLong; + @visibleForTesting + int? get debugLengthUnicodeCodePointsIfLong => _lengthUnicodeCodePointsIfLong; + int? _computeLengthUnicodeCodePointsIfLong() => + _textNormalized.length > maxLengthUnicodeCodePoints + ? _textNormalized.runes.length + : null; + List get validationErrors => _validationErrors; late List _validationErrors; List _computeValidationErrors(); @@ -40,6 +61,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 +97,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 +112,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, ]; } @@ -125,6 +150,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; @@ -266,11 +294,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 ec27585e88..96bc6cb837 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -256,20 +256,17 @@ void main() { await checkErrorResponse(tester); }); - // TODO(#1238) unskip - // testWidgets('max-length content not rejected', (tester) async { - // await prepareWithContent(tester, - // makeStringWithCodePoints(kMaxMessageLengthCodePoints)); - // await tapSendButton(tester); - // checkNoErrorDialog(tester); - // }); - - // TODO(#1238) replace with above commented-out test - testWidgets('some content not rejected', (tester) async { - await prepareWithContent(tester, 'a' * kMaxMessageLengthCodePoints); + testWidgets('max-length content not rejected', (tester) async { + await prepareWithContent(tester, + makeStringWithCodePoints(kMaxMessageLengthCodePoints)); await tapSendButton(tester); checkNoErrorDialog(tester); }); + + testWidgets('code points not counted unnecessarily', (tester) async { + await prepareWithContent(tester, 'a' * kMaxMessageLengthCodePoints); + check(controller!.content.debugLengthUnicodeCodePointsIfLong).isNull(); + }); }); group('topic', () { @@ -296,20 +293,18 @@ void main() { await checkErrorResponse(tester); }); - // TODO(#1238) unskip - // testWidgets('max-length topic not rejected', (tester) async { - // await prepareWithTopic(tester, - // makeStringWithCodePoints(kMaxTopicLengthCodePoints)); - // await tapSendButton(tester); - // checkNoErrorDialog(tester); - // }); - - // TODO(#1238) replace with above commented-out test - testWidgets('some topic not rejected', (tester) async { - await prepareWithTopic(tester, 'a' * kMaxTopicLengthCodePoints); + testWidgets('max-length topic not rejected', (tester) async { + await prepareWithTopic(tester, + makeStringWithCodePoints(kMaxTopicLengthCodePoints)); await tapSendButton(tester); checkNoErrorDialog(tester); }); + + testWidgets('code points not counted unnecessarily', (tester) async { + await prepareWithTopic(tester, 'a' * kMaxTopicLengthCodePoints); + check((controller as StreamComposeBoxController) + .topic.debugLengthUnicodeCodePointsIfLong).isNull(); + }); }); });