Repository: superlistapp/super_editor (dart)

Issue:
[Super Editor][Attributed Text] - Inline placeholder breaking attribution behavior
We have a report from a client about an error that occurs in an `AttributedText` when there's only a single placeholder, along with an `Attribution` around that placeholder.

The editor action in question is the following:

```dart
InsertAttributedTextRequest(
            caret,
            AttributedText(
                '',
                AttributedSpans(
                  attributions: [
                    SpanMarker(
                      attribution: CustomEmojiAttribution(emoji),
                      offset: 0,
                      markerType: SpanMarkerType.start,
                    ),
                    SpanMarker(
                      attribution: CustomEmojiAttribution(emoji),
                      offset: 0,
                      markerType: SpanMarkerType.end,
                    ),
                  ],
                ),
                {
                  0: emoji,
                }),
          )
```

Notice that the `AttributedText` is empty except for a placeholder at position 0. But there's an attribution applied around that placeholder.

The reported error stacktrace is:

```
======== Exception caught by services library ======================================================

The following _Exception was thrown during method call TextInputClient.updateEditingStateWithDeltas:

Exception: Another AttributedSpans can only be appended after the final marker in this AttributedSpans. Final marker: [SpanMarker] - attribution: Instance of 'CustomEmojiAttribution', offset: 1, type: SpanMarkerType.end



When the exception was thrown, this was the stack: 

#0      AttributedSpans.addAt (package:attributed_text/src/attributed_spans.dart:714:7)

#1      AttributedText.copyAndAppend (package:attributed_text/src/attributed_text.dart:505:21)

#2      AttributedText.insertString (package:attributed_text/src/attributed_text.dart:555:22)

#3      InsertTextCommand.execute (package:super_editor/src/default_editor/text.dart:2007:27)

#4      _DocumentEditorCommandExecutor.executeCommand (package:super_editor/src/core/editor.dart:701:15)

#5      Editor._executeCommand (package:super_editor/src/core/editor.dart:304:22)

#6      Editor.execute (package:super_editor/src/core/editor.dart:266:30)

#7      TextDeltasDocumentEditor._insertPlainText (package:super_editor/src/default_editor/document_ime/document_delta_editing.dart:331:12)

#8      TextDeltasDocumentEditor.insert (package:super_editor/src/default_editor/document_ime/document_delta_editing.dart:283:23)

#9      TextDeltasDocumentEditor._applyInsertion (package:super_editor/src/default_editor/document_ime/document_delta_editing.dart:180:5)

#10     TextDeltasDocumentEditor.applyDeltas (package:super_editor/src/default_editor/document_ime/document_delta_editing.dart:81:9)

#11     DocumentImeInputClient.updateEditingValueWithDeltas (package:super_editor/src/default_editor/document_ime/document_ime_communication.dart:232:30)

#12     TextInput._handleTextInputInvocation (package:flutter/src/services/text_input.dart:1912:63)

#13     TextInput._loudlyHandleTextInputInvocation (package:flutter/src/services/text_input.dart:1803:20)

#14     MethodChannel._handleAsMethodCall (package:flutter/src/services/platform_channel.dart:571:55)

#15     MethodChannel.setMethodCallHandler.<anonymous closure> (package:flutter/src/services/platform_channel.dart:564:34)

#16     _DefaultBinaryMessenger.setMessageHandler.<anonymous closure> (package:flutter/src/services/binding.dart:622:35)

#17     _invoke2.<anonymous closure> (dart:ui/hooks.dart:347:15)

#21     _invoke2 (dart:ui/hooks.dart:346:10)

#22     _ChannelCallbackRecord.invoke (dart:ui/channel_buffers.dart:45:5)

#23     _Channel.push (dart:ui/channel_buffers.dart:135:31)

#24     ChannelBuffers.push (dart:ui/channel_buffers.dart:343:17)

#25     PlatformDispatcher._dispatchPlatformMessage (dart:ui/platform_dispatcher.dart:758:22)

#26     _dispatchPlatformMessage (dart:ui/hooks.dart:257:31)

(elided 3 frames from dart:async)

call: MethodCall(TextInputClient.updateEditingStateWithDeltas, [13, {deltas: [{selectionBase: 4, oldText: . ￼, selectionAffinity: TextAffinity.downstream, deltaEnd: 3, deltaText:  , deltaStart: 3, composingExtent: -1, selectionIsDirectional: false, selectionExtent: 4, composingBase: -1}]}])

====================================================================================================
```

Pull request description:
[SuperEditor][AttributedText] Fix placeholder breaking attribution behavior (Resolves #2565)

Inserting an empty text with a placeholder, with an attribution around it, causes the editor to crash when trying to type.

```console
════════ Exception caught by services library ══════════════════════════════════
The following _Exception was thrown during method call TextInputClient.updateEditingStateWithDeltas:
Exception: Another AttributedSpans can only be appended after the final marker in this AttributedSpans. Final marker: [SpanMarker] - attribution: [NamedAttribution]: bold, offset: 1, type: SpanMarkerType.end

When the exception was thrown, this was the stack:
#0      AttributedSpans.addAt (package:attributed_text/src/attributed_spans.dart:714:7)
#1      AttributedText.copyAndAppend (package:attributed_text/src/attributed_text.dart:510:21)
#2      AttributedText.insertString (package:attributed_text/src/attributed_text.dart:560:22)
#3      InsertTextCommand.execute (package:super_editor/src/default_editor/text.dart:2007:27)
#4      _DocumentEditorCommandExecutor.executeCommand (package:super_editor/src/core/editor.dart:701:15)
#5      Editor._executeCommand (package:super_editor/src/core/editor.dart:304:22)
#6      Editor.execute (package:super_editor/src/core/editor.dart:266:30)
#7      TextDeltasDocumentEditor._insertPlainText (package:super_editor/src/default_editor/document_ime/document_delta_editing.dart:331:12)
#8      TextDeltasDocumentEditor.insert (package:super_editor/src/default_editor/document_ime/document_delta_editing.dart:283:23)
#9      TextDeltasDocumentEditor._applyInsertion (package:super_editor/src/default_editor/document_ime/document_delta_editing.dart:180:5)
#10     TextDeltasDocumentEditor.applyDeltas (package:super_editor/src/default_editor/document_ime/document_delta_editing.dart:81:9)
#11     DocumentImeInputClient.updateEditingValueWithDeltas (package:super_editor/src/default_editor/document_ime/document_ime_communication.dart:232:30)
#12     TextInput._handleTextInputInvocation (package:flutter/src/services/text_input.dart:2005:63)
#13     TextInput._loudlyHandleTextInputInvocation (package:flutter/src/services/text_input.dart:1874:20)
#14     MethodChannel._handleAsMethodCall (package:flutter/src/services/platform_channel.dart:611:55)
#15     MethodChannel.setMethodCallHandler.<anonymous closure> (package:flutter/src/services/platform_channel.dart:601:55)
#16     _DefaultBinaryMessenger.setMessageHandler.<anonymous closure> (package:flutter/src/services/binding.dart:650:35)
#17     _invoke2 (dart:ui/hooks.dart:348:13)
#18     _ChannelCallbackRecord.invoke (dart:ui/channel_buffers.dart:45:5)
#19     _Channel.push (dart:ui/channel_buffers.dart:136:31)
#20     ChannelBuffers.push (dart:ui/channel_buffers.dart:344:17)
#21     PlatformDispatcher._dispatchPlatformMessage (dart:ui/platform_dispatcher.dart:786:22)
#22     _dispatchPlatformMessage (dart:ui/hooks.dart:262:31)

call: MethodCall(TextInputClient.updateEditingStateWithDeltas, [1, {deltas: [{selectionBase: 4, oldText: . ￼, selectionAffinity: TextAffinity.downstream, deltaEnd: 3, deltaText: f, deltaStart: 3, composingExtent: -1, selectionIsDirectional: false, selectionExtent: 4, composingBase: -1}]}])
════════════════════════════════════════════════════════════════════════════════
```

The issue lives in `AttributedText.copyText`: 

```dart
AttributedText copyText(int startOffset, [int? endOffset]) {
    _log.fine('start: $startOffset, end: $endOffset');

    final placeholdersBeforeStartOffset = placeholders.entries.where((entry) => entry.key < startOffset);
    final textStartCopyOffset = startOffset - placeholdersBeforeStartOffset.length;

    final placeholdersAfterStartBeforeEndOffset = placeholders.entries.where(
      (entry) => startOffset <= entry.key && entry.key < (endOffset ?? length),
    );
    final textEndCopyOffset =
        (endOffset ?? length) - placeholdersBeforeStartOffset.length - placeholdersAfterStartBeforeEndOffset.length;

    // Note: -1 because copyText() uses an exclusive `start` and `end` but
    // _copyAttributionRegion() uses an inclusive `start` and `end`.
    final startCopyOffset = startOffset < _text.length ? startOffset : _text.length - 1; <---- this line

    // more code...
}
```

We are using the text without placeholders to determine the starting offset to copy the spans, but the span marker offsets are based on the text with the placeholders. 

The current implementation causes the `startCopyOffset` to be `-1`. This causes an issue in `AttributedSpans.copyAttributionRegion`, in the following code:

```dart
// Directly copy every marker that appears within the cut
// region.
_markers //
    .where((marker) => startOffset <= marker.offset && marker.offset <= endOffset!) //
    .forEach((marker) {
  _log.fine('copying "${marker.attribution}" at ${marker.offset} from original AttributionSpans to copy region.');
  cutAttributions.add(marker.copyWith(
    offset: marker.offset - startOffset, <---- this line
  ));
});
```

The expression `marker.offset - startOffset` evaluates to `0 - (-1) = 1`. The span marker should be at offset 0, and we are changing it to offset 1. This is the cause of the exception.

This PR fixes this issue by computing the start and end offset of the span copy to be based on the text with placeholders.

Patch:
diff --git a/attributed_text/lib/src/attributed_text.dart b/attributed_text/lib/src/attributed_text.dart
index 11e77b52a0..82403bf726 100644
--- a/attributed_text/lib/src/attributed_text.dart
+++ b/attributed_text/lib/src/attributed_text.dart
@@ -421,16 +421,20 @@ class AttributedText {
     final textEndCopyOffset =
         (endOffset ?? length) - placeholdersBeforeStartOffset.length - placeholdersAfterStartBeforeEndOffset.length;
 
+    // The span marker offsets are based on the text with placeholders, so we need
+    // to copy the text with placeholders to ensure the span markers are correct.
+    final textWithPlaceholders = toPlainText();
+
     // Note: -1 because copyText() uses an exclusive `start` and `end` but
     // _copyAttributionRegion() uses an inclusive `start` and `end`.
-    final startCopyOffset = startOffset < _text.length ? startOffset : _text.length - 1;
+    final startCopyOffset = startOffset < textWithPlaceholders.length ? startOffset : textWithPlaceholders.length - 1;
     int endCopyOffset;
     if (endOffset == startOffset) {
       endCopyOffset = startCopyOffset;
     } else if (endOffset != null) {
       endCopyOffset = endOffset - 1;
     } else {
-      endCopyOffset = _text.length - 1;
+      endCopyOffset = textWithPlaceholders.length - 1;
     }
     _log.fine('offsets, start: $startCopyOffset, end: $endCopyOffset');
 
diff --git a/super_editor/pubspec.yaml b/super_editor/pubspec.yaml
index 9c8f930386..ae56de45bd 100644
--- a/super_editor/pubspec.yaml
+++ b/super_editor/pubspec.yaml
@@ -48,11 +48,11 @@ dependencies:
   flutter_test_robots: ^0.0.24
   clock: ^1.1.1
 
-#dependency_overrides:
+dependency_overrides:
   # Override to local mono-repo path so devs can test this repo
   # against changes that they're making to other mono-repo packages
-#  attributed_text:
-#    path: ../attributed_text
+  attributed_text:
+    path: ../attributed_text
 #  super_text_layout:
 #    path: ../super_text_layout
 


Test patch:
diff --git a/attributed_text/test/attributed_text_placeholders_test.dart b/attributed_text/test/attributed_text_placeholders_test.dart
index f17f2a00dc..1934128706 100644
--- a/attributed_text/test/attributed_text_placeholders_test.dart
+++ b/attributed_text/test/attributed_text_placeholders_test.dart
@@ -420,6 +420,46 @@ void main() {
           }),
         );
       });
+
+      test("empty text with leading placeholder (with attributions)", () {
+        // Create an empty text containing a placeholder with an attribution around it.
+        final text = AttributedText(
+          '',
+          AttributedSpans(
+            attributions: [
+              const SpanMarker(
+                attribution: _bold,
+                offset: 0,
+                markerType: SpanMarkerType.start,
+              ),
+              const SpanMarker(
+                attribution: _bold,
+                offset: 0,
+                markerType: SpanMarkerType.end,
+              ),
+            ],
+          ),
+          {
+            0: const _FakePlaceholder('leading'),
+          },
+        );
+
+        expect(
+          text.copyText(0),
+          AttributedText(
+            "",
+            AttributedSpans(
+              attributions: const [
+                SpanMarker(attribution: _bold, offset: 0, markerType: SpanMarkerType.start),
+                SpanMarker(attribution: _bold, offset: 0, markerType: SpanMarkerType.end),
+              ],
+            ),
+            {
+              0: const _FakePlaceholder("leading"),
+            },
+          ),
+        );
+      });
     });
 
     group("copy and append >", () {
diff --git a/super_editor/test/super_editor/text_entry/inline_widgets_test.dart b/super_editor/test/super_editor/text_entry/inline_widgets_test.dart
index 1bed3a4fa8..a6afd58211 100644
--- a/super_editor/test/super_editor/text_entry/inline_widgets_test.dart
+++ b/super_editor/test/super_editor/text_entry/inline_widgets_test.dart
@@ -133,6 +133,58 @@ void main() {
         ),
       );
     });
+
+    testWidgetsOnArbitraryDesktop("can insert an inline widget with attributions in an empty paragraph",
+        (tester) async {
+      final editor = await _pumpScaffold(tester);
+
+      // Insert an empty text containing a placeholder with an attribution around it.
+      editor.execute([
+        InsertStyledTextAtCaretRequest(
+          AttributedText(
+              '',
+              AttributedSpans(
+                attributions: [
+                  const SpanMarker(
+                    attribution: _emojiAttribution,
+                    offset: 0,
+                    markerType: SpanMarkerType.start,
+                  ),
+                  const SpanMarker(
+                    attribution: _emojiAttribution,
+                    offset: 0,
+                    markerType: SpanMarkerType.end,
+                  ),
+                ],
+              ),
+              {
+                0: const _TestPlaceholder(),
+              }),
+        ),
+      ]);
+      await tester.pump();
+
+      // Ensure we can type after the inline placeholder was added.
+      await tester.typeImeText('hello');
+
+      // Ensure the inline widget was kept, the text was inserted, and the attribution
+      // was not applied to the inserted characters.
+      expect(
+        SuperEditorInspector.findTextInComponent('1'),
+        AttributedText(
+          'hello',
+          AttributedSpans(
+            attributions: const [
+              SpanMarker(attribution: _emojiAttribution, offset: 0, markerType: SpanMarkerType.start),
+              SpanMarker(attribution: _emojiAttribution, offset: 0, markerType: SpanMarkerType.end),
+            ],
+          ),
+          {
+            0: const _TestPlaceholder(),
+          },
+        ),
+      );
+    });
   });
 }
 
@@ -165,3 +217,5 @@ Widget? _buildInlineTestWidget(BuildContext context, TextStyle style, Object pla
 class _TestPlaceholder {
   const _TestPlaceholder();
 }
+
+const _emojiAttribution = NamedAttribution('emoji');