[SuperEditor] Fix text duplicated when a placeholder is inserted in a text with attributions (Resolves #2595)

[angelosilvestre]

[SuperEditor] Fix text duplicated when a placeholder is inserted in a text with attributions (Resolves #2595)

When inserting a placeholder after an attributed region, the editor duplicates the text:

Nagranie.z.ekranu.2025-02-27.o.11.43.02.mov

The node's text itself isn't duplicated, the issue lives in the InlineSpan computation:

super_editor/super_editor/lib/src/infrastructure/attributed_text_styles.dart

Lines 64 to 81 in 068aeb0

	} else {
	// This section is text. The end of this text is either the
	// end of the AttributedText, or the index of the next placeholder.
	contentEnd = span.end + 1;
	for (final entry in placeholders.entries) {
	if (entry.key > start) {
	contentEnd = entry.key;
	break;
	}
	}
	inlineSpans.add(
	TextSpan(
	text: substring(start, contentEnd),
	style: styleBuilder(span.attributions),
	),
	);
	}

When there is a placeholder, we are assuming that the placeholder belongs in the current text span, which isn't always the case when there are attributions before the placeholder.

This PR fixes the issue by checking if the placeholder isn't part of a subsequent span.

I added tests for the computeInlineSpan extension method, but we could add tests using SuperEditor if needed.

I also found a separate issue when trying to insert a placeholder if the inline markdown plugin converter is used. I filed #2604 for that.

[matthew-carroll]

Did you intend for the "bold" attribution to apply to [xWelcom]e or did you mean for it to be x[Welcome]?

[angelosilvestre]

I intended it to be x[Welcome]. I forgot to adjust the offsets to account for the placeholder.

[matthew-carroll]

We should find some other way to describe this test and the expectation. It's not clear what "modification" might happen, or that we're guarding against.

[angelosilvestre]

The test was modified.

[matthew-carroll]

I think these indices are also incorrect. With the placeholder at index 10, "SuperEditor" should start at index 12.

[angelosilvestre]

Yeah, updated.

[matthew-carroll]

These tests should probably go with the other placeholder tests. Do we have an existing test suite for those in Super Editor? Or did I only add them to attributed_text?

Also, in these tests you verify the plain text, but given the nuanced nature of computing the inline span, we should probably verify the entire inline span structure. Is that a reasonable effort?

[angelosilvestre]

Updated.

[angelosilvestre]

Do we have an existing test suite for those in Super Editor? Or did I only add them to attributed_text?

I didn't find any test suite for placeholders in SuperEditor, only in attributed_text.

[matthew-carroll]

I'm wondering if this whole if/else should be reworked....

It looks like when I wrote it, I treated the placeholder at the beginning of a span as a special case, but it looks like you've discovered that it's not a special case, right?

So the original if/else essentially thought about spans like this:

[|x|and some text]
// or just
[and some text]

But this PR seems to suggest that we might have:

[|x| and |x| and |x|]

Where "[ ]" represents a singular group of attributions, such as a single bold attribution.

If my understanding is correct, then I think we probably don't need an if/else at all. Instead, we should take the span and move through it from left to right, looking for any number of placeholders and slicing it up. No need to create a special condition for when a placeholder appears at the start.

Such a loop would take:

[|x| and |x| and |x|]

and chop it into:

|x| " and " |x| " and " |x|

I also noticed that we're building a text style with zero attributions for placeholders, but that's probably wrong? Attributions like bold are irrelevant for placeholders, but something like a font size attribution might be important. So we should probably use the same base text style for the placeholders as we do for the text in the overall span.

[angelosilvestre]

Updated.

[matthew-carroll]

Please add to this comment a description of the action you're taking. You've found a placeholder....and therefore what?