[Attributed Text] - Fix: A few methods still (erroneously) using plain text length without accounting for placeholders (Resolves #2515) (#2516)

Author: matthew-carroll

attributed_text/lib/src/attributed_text.dart

@@ -270,14 +270,14 @@ class AttributedText {
   /// Returns all spans in this [AttributedText] for the given [attributions].
   Set<AttributionSpan> getAttributionSpans(Set<Attribution> attributions) => getAttributionSpansInRange(
         attributionFilter: (a) => attributions.contains(a),
-        range: SpanRange(0, _text.length),
+        range: SpanRange(0, length),
       );
 
   /// Returns all spans in this [AttributedText], for attributions that are
   /// selected by the given [filter].
   Set<AttributionSpan> getAttributionSpansByFilter(AttributionFilter filter) => getAttributionSpansInRange(
         attributionFilter: filter,
-        range: SpanRange(0, _text.length),
+        range: SpanRange(0, length),
       );
 
   /// Returns spans for each attribution that (at least partially) appear
@@ -502,7 +502,7 @@ class AttributedText {
 
     return AttributedText(
       _text + other._text,
-      spans.copy()..addAt(other: other.spans, index: _text.length),
+      spans.copy()..addAt(other: other.spans, index: length),
       {
         ...placeholders,
         ...other.placeholders.map((offset, placeholder) => MapEntry(offset + length, placeholder)),
@@ -707,7 +707,7 @@ class AttributedText {
   ///
   /// Attribution groups are useful when computing all style variations for [AttributedText].
   Iterable<MultiAttributionSpan> computeAttributionSpans() {
-    return spans.collapseSpans(contentLength: _text.length);
+    return spans.collapseSpans(contentLength: length);
   }
 
   /// Returns a copy of this [AttributedText].

attributed_text/test/attributed_text_placeholders_test.dart

@@ -247,6 +247,34 @@ void main() {
       });
     });
 
+    group("attribution queries >", () {
+      test('finds spans that exceed plain text length', () {
+        final attributedText = AttributedText(
+          'Hello world',
+          AttributedSpans(
+            attributions: [
+              const SpanMarker(attribution: ExpectedSpans.bold, offset: 0, markerType: SpanMarkerType.start),
+              const SpanMarker(attribution: ExpectedSpans.bold, offset: 12, markerType: SpanMarkerType.end),
+            ],
+          ),
+          {
+            11: const _FakePlaceholder("trailing1"),
+            12: const _FakePlaceholder("trailing2"),
+          },
+        );
+
+        final ranges = attributedText.getAttributionSpans({ExpectedSpans.bold});
+
+        expect(ranges.length, 1);
+        expect(
+          ranges,
+          [
+            const AttributionSpan(attribution: ExpectedSpans.bold, start: 0, end: 12),
+          ],
+        );
+      });
+    });
+
     group("full copy >", () {
       test("only a single placeholder", () {
         expect(
@@ -463,6 +491,66 @@ void main() {
           }),
         );
       });
+
+      test("at end of text (with attributions) with leading placeholder", () {
+        // Note: We include attributions in this test because in a client app the presence
+        //       of attributions unearthed a bug where we were still using the plain text
+        //       length, when we should have been using the text + placeholders length.
+        expect(
+          AttributedText(
+            "Hello",
+            AttributedSpans(attributions: const [
+              SpanMarker(attribution: ExpectedSpans.bold, offset: 1, markerType: SpanMarkerType.start),
+              SpanMarker(attribution: ExpectedSpans.bold, offset: 5, markerType: SpanMarkerType.end),
+            ]),
+            {
+              0: const _FakePlaceholder("leading"),
+            },
+          ).copyAndAppend(
+            AttributedText(", world!"),
+          ),
+          AttributedText(
+            "Hello, world!",
+            AttributedSpans(attributions: const [
+              SpanMarker(attribution: ExpectedSpans.bold, offset: 1, markerType: SpanMarkerType.start),
+              SpanMarker(attribution: ExpectedSpans.bold, offset: 5, markerType: SpanMarkerType.end),
+            ]),
+            {
+              0: const _FakePlaceholder("leading"),
+            },
+          ),
+        );
+      });
+
+      test("at end of text (with attributions) with trailing placeholder", () {
+        // Note: We include attributions in this test because in a client app the presence
+        //       of attributions unearthed a bug where we were still using the plain text
+        //       length, when we should have been using the text + placeholders length.
+        expect(
+          AttributedText(
+            "Hello",
+            AttributedSpans(attributions: const [
+              SpanMarker(attribution: ExpectedSpans.bold, offset: 0, markerType: SpanMarkerType.start),
+              SpanMarker(attribution: ExpectedSpans.bold, offset: 4, markerType: SpanMarkerType.end),
+            ]),
+            {
+              5: const _FakePlaceholder("trailing"),
+            },
+          ).copyAndAppend(
+            AttributedText(", world!"),
+          ),
+          AttributedText(
+            "Hello, world!",
+            AttributedSpans(attributions: const [
+              SpanMarker(attribution: ExpectedSpans.bold, offset: 0, markerType: SpanMarkerType.start),
+              SpanMarker(attribution: ExpectedSpans.bold, offset: 4, markerType: SpanMarkerType.end),
+            ]),
+            {
+              5: const _FakePlaceholder("trailing"),
+            },
+          ),
+        );
+      });
     });
 
     test("insert attributed text >", () {
@@ -576,6 +664,38 @@ void main() {
         ),
       );
     });
+
+    group("collapses spans >", () {
+      test("when placeholders sit beyond plain text", () {
+        final attributedText = AttributedText(
+          'Hello world',
+          AttributedSpans(
+            attributions: [
+              const SpanMarker(attribution: ExpectedSpans.bold, offset: 0, markerType: SpanMarkerType.start),
+              const SpanMarker(attribution: ExpectedSpans.bold, offset: 11, markerType: SpanMarkerType.end),
+              const SpanMarker(attribution: ExpectedSpans.underline, offset: 12, markerType: SpanMarkerType.start),
+              const SpanMarker(attribution: ExpectedSpans.underline, offset: 12, markerType: SpanMarkerType.end),
+            ],
+          ),
+          {
+            11: const _FakePlaceholder("trailing1"),
+            12: const _FakePlaceholder("trailing2"),
+          },
+        );
+
+        final spans = attributedText.computeAttributionSpans().toList();
+
+        expect(spans.length, 2);
+
+        expect(spans[0].attributions, {ExpectedSpans.bold});
+        expect(spans[0].start, 0);
+        expect(spans[0].end, 11);
+
+        expect(spans[1].attributions, {ExpectedSpans.underline});
+        expect(spans[1].start, 12);
+        expect(spans[1].end, 12);
+      });
+    });
   });
 }
 

attributed_text/test/attributed_text_test.dart

@@ -21,7 +21,7 @@ void main() {
         applyAttributions: {ExpectedSpans.bold},
       );
 
-      expect(newText.text, 'aabcdefghij');
+      expect(newText.toPlainText(), 'aabcdefghij');
       expect(
         newText.hasAttributionsWithin(attributions: {ExpectedSpans.bold}, range: const SpanRange(0, 10)),
         true,
@@ -43,7 +43,7 @@ void main() {
         );
 
         final slice = text.copyTextInRange(const SpanRange(5, 9));
-        expect(slice.text, "that");
+        expect(slice.toPlainText(), "that");
         expect(slice.length, 4);
         expect(slice.getAttributedRange({ExpectedSpans.bold}, 0), const SpanRange(0, 1));
         expect(slice.getAttributedRange({ExpectedSpans.italics}, 2), const SpanRange(2, 3));
@@ -63,7 +63,7 @@ void main() {
         );
 
         final slice = text.copyText(5, 9);
-        expect(slice.text, "that");
+        expect(slice.toPlainText(), "that");
         expect(slice.length, 4);
         expect(slice.getAttributedRange({ExpectedSpans.bold}, 0), const SpanRange(0, 1));
         expect(slice.getAttributedRange({ExpectedSpans.italics}, 2), const SpanRange(2, 3));