compose: Enforce max topic/content length by code points, following API

Fixes zulip#1238
Gaurav-Kushwaha-1225 · Jan 23, 2025 · a23309a · a23309a
1 parent e67b014
commit a23309a
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 32 deletions.
diff --git 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<ErrorT> 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<ErrorT> get validationErrors => _validationErrors;
   late List<ErrorT> _validationErrors;
   List<ErrorT> _computeValidationErrors();
@@ -40,6 +61,8 @@ abstract class ComposeController<ErrorT> 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<TopicValidationError> {
   //   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<TopicValidationError> {
       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<ContentValidationError>
     _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<ContentValidationError>
       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
@@ -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();
+      });
     });
   });