[BUG] - Exception when deleting composing action tag, then starting new one

[KevinBrendel]

Package Version
super_editor, GitHub, main branch

User Info
coneno, QuikFlow

To Reproduce
Steps to reproduce the behavior:

1. Go to the "Action Tags" section of the example app
2. Type some text
3. Place the cursor in the middle of the text
4. Type "/"
5. Press backspace
6. Type "/" again
7. See exception

Actual behavior

════════ Exception caught by services library ══════════════════════════════════
The following _Exception was thrown during method call TextInputClient.updateEditingStateWithDeltas:
Exception: removeAttribution() did not satisfy start < 0 and start > end, start: 6, end: 5

When the exception was thrown, this was the stack:
#0      AttributedSpans.removeAttribution (package:attributed_text/src/attributed_spans.dart:514:7)
attributed_spans.dart:514
#1      RemoveTextAttributionsCommand.execute (package:super_editor/src/default_editor/text.dart:1444:15)
text.dart:1444
#2      _DocumentEditorCommandExecutor.executeCommand (package:super_editor/src/core/editor.dart:701:15)
editor.dart:701
#3      Editor._executeCommand (package:super_editor/src/core/editor.dart:304:22)
editor.dart:304
#4      Editor.execute (package:super_editor/src/core/editor.dart:266:30)
editor.dart:266
#5      ActionTagComposingReaction._healCancelledTags (package:super_editor/src/default_editor/text_tokenizing/action_tags.dart:364:23)
action_tags.dart:364
#6      ActionTagComposingReaction.react (package:super_editor/src/default_editor/text_tokenizing/action_tags.dart:286:5)
action_tags.dart:286
#7      Editor._reactToChanges (package:super_editor/src/core/editor.dart:357:16)
editor.dart:357
#8      Editor.endTransaction (package:super_editor/src/core/editor.dart:222:5)
editor.dart:222
#9      TextDeltasDocumentEditor.applyDeltas (package:super_editor/src/default_editor/document_ime/document_delta_editing.dart:118:12)
document_delta_editing.dart:118
#10     DocumentImeInputClient.updateEditingValueWithDeltas (package:super_editor/src/default_editor/document_ime/document_ime_communication.dart:232:30)
document_ime_communication.dart:232
#11     DeltaTextInputClientDecorator.updateEditingValueWithDeltas (package:super_editor/src/default_editor/document_ime/ime_decoration.dart:129:14)
ime_decoration.dart:129
#12     TextInput._handleTextInputInvocation (package:flutter/src/services/text_input.dart:1906:63)
text_input.dart:1906
#13     TextInput._loudlyHandleTextInputInvocation (package:flutter/src/services/text_input.dart:1794:20)
text_input.dart:1794
#14     MethodChannel._handleAsMethodCall (package:flutter/src/services/platform_channel.dart:571:55)
platform_channel.dart:571
#15     MethodChannel.setMethodCallHandler.<anonymous closure> (package:flutter/src/services/platform_channel.dart:564:34)
platform_channel.dart:564
#16     _DefaultBinaryMessenger.setMessageHandler.<anonymous closure> (package:flutter/src/services/binding.dart:581:35)
binding.dart:581
#17     _invoke2 (dart:ui/hooks.dart:344:13)
hooks.dart:344
#18     _ChannelCallbackRecord.invoke (dart:ui/channel_buffers.dart:45:5)
channel_buffers.dart:45
#19     _Channel.push (dart:ui/channel_buffers.dart:135:31)
channel_buffers.dart:135
#20     ChannelBuffers.push (dart:ui/channel_buffers.dart:343:17)
channel_buffers.dart:343
#21     PlatformDispatcher._dispatchPlatformMessage (dart:ui/platform_dispatcher.dart:750:22)
platform_dispatcher.dart:750
#22     _dispatchPlatformMessage (dart:ui/hooks.dart:257:31)
hooks.dart:257

call: MethodCall(TextInputClient.updateEditingStateWithDeltas, [7, {deltas: [{selectionBase: 8, oldText: . asdfasdf, selectionAffinity: TextAffinity.downstream, deltaEnd: 7, deltaText: /, deltaStart: 7, composingExtent: -1, selectionIsDirectional: false, selectionExtent: 8, composingBase: -1}]}])
════════════════════════════════════════════════════════════════════════════════

Expected behavior
A new action tag is being composed.

Platform
macOS

Flutter version
Flutter 3.22.3 • channel stable • https://github.com/flutter/flutter.git
Framework • revision b0850beeb2 (4 weeks ago) • 2024-07-16 21:43:41 -0700
Engine • revision 235db911ba
Tools • Dart 3.4.4 • DevTools 2.34.3

Screenshots

[KevinBrendel]

@matthew-carroll
The cause for this and for #2241 as well as #2242 appears to be that _cancelComposingTag uses _composingTag as a reference, which can be outdated if _cancelComposingTag is called because tagAroundPosition is null.

Fixed it by directly using the actionTagComposingAttribution range(s) as a reference for now:
coneno@e95e848

Could of course be more optimized, in case there can actually only be one range, even if inserting a space, etc.

Needs further testing, but so far this implementation appears to fix all three issues.

[angelosilvestre]

@matthew-carroll I'm seeing a behavior that I'm not sure is expected. Pressing "/" at the beginning of a word causes the word to be converted to a composing tag:

Screen.Recording.2024-10-14.at.20.38.48.mov

I wouldn't expect that. Could you confirm if this is desired? This might influence the fix for this bug.

[matthew-carroll]

@angelosilvestre I don't remember. Is there a test that does that?

[angelosilvestre]

@matthew-carroll I didn't find any tests for that.

[matthew-carroll]

@angelosilvestre then let's assume that behavior isn't expected

[alterhuman]

@angelosilvestre @matthew-carroll the behaviour in most apps with action tags is this:

1. Pressing / at the beginning of a word should create it to a composing tag but there should be no text passed for matching with actions.
2. The text is only matched till caret position. Aka the letters between the / and the caret.
3. If there is a space character " " between the / and the caret, the popover is hidden without removing the /.

This is how it works in Obsidian and Craft. In Superlist, they don't wait till the " ", they just hide the popover when there are no actions matching with the text between / and the caret. But the behaviour of matching till the caret is same.
Doing this doesn't affect the normal action tags behaviour while also fixing the current bug.

This is the updated code for findTagAroundPosition with the fix:

static TagAroundPosition? findTagAroundPosition({
    required TagRule tagRule,
    required String nodeId,
    required AttributedText text,
    required TextNodePosition expansionPosition,
    required bool Function(Set<Attribution> tokenAttributions) isTokenCandidate,
  }) {
    final rawText = text.text;
    if (rawText.isEmpty) {
      return null;
    }

    int splitIndex = min(expansionPosition.offset, rawText.length);
    splitIndex = max(splitIndex, 0);

    // Get the characters before the caret position
    final charactersBefore = rawText.substring(0, splitIndex).characters;
    final iteratorUpstream = charactersBefore.iteratorAtEnd;

    // If the character before the caret is an excluded character, return null
    if (charactersBefore.isNotEmpty && tagRule.excludedCharacters.contains(charactersBefore.last)) {
      return null;
    }

    // Move upstream to find the trigger character
    bool foundTrigger = false;

    while (iteratorUpstream.moveBack()) {
      final currentCharacter = iteratorUpstream.current;

      if (currentCharacter == tagRule.trigger) {
        foundTrigger = true;
        break;
      } else if (tagRule.excludedCharacters.contains(currentCharacter)) {
        return null;
      } else if (currentCharacter == " ") {
        return null; // Space found; close the dropdown
      }
    }

    if (!foundTrigger) {
      return null;
    }

    final tokenStartOffset = iteratorUpstream.stringBeforeLength;
    final tokenRange = SpanRange(tokenStartOffset, splitIndex);

    final tagText = text.substringInRange(tokenRange);
    if (!tagText.startsWith(tagRule.trigger)) {
      return null;
    }

    final tokenAttributions = text.getAttributionSpansInRange(attributionFilter: (a) => true, range: tokenRange);
    if (!isTokenCandidate(tokenAttributions.map((span) => span.attribution).toSet())) {
      return null;
    }

    return TagAroundPosition(
      indexedTag: IndexedTag(
        Tag(tagRule.trigger, tagText.substring(1)),
        nodeId,
        tokenStartOffset,
      ),
      searchOffset: expansionPosition.offset,
    );
  }

This alongside the fix added by @KevinBrendel completely fixes this issue.

[KevinBrendel]

I think there was a small index error in my original fix, though I am not sure if it can make a difference in practice:
c0a6a3b

[angelosilvestre]

The text is only matched till caret position. Aka the letters between the / and the caret.

@alterhuman It seems that updating findTagAroundPosition to do this already fixes all the issues. I'll try to write some tests and put up a PR.

[alterhuman]

@angelosilvestre the issue of caret getting stuck in the middle is not getting fixed for me by just update findTagAroundPosition.

Also need to implement this: coneno@e95e848. This minor fix of index calculation didn't do anything but not bad to fix it: c0a6a3b