Migrations: Convert all sibling RTE blocks (closes #22979)

[AndyButland]

Description

This PR fixes a v13 → v17 upgrade bug where multiple direct-sibling <umb-rte-block> elements in RTE markup were collapsed into a single match by the migration regex, leaving all-but-last sibling UDIs un-converted (data-content-key="umb://element/..." rather than the hyphenated GUID).

The cause was a greedy .* on both sides of the attribute capture in ConvertRichTextEditorProperties.BlockRegex() (and its verbatim copy in LocalLinkRteProcessor). Replaced with a non-greedy, attribute-scoped pattern that matches each block independently.

I've also consolidated the regex + replacement lambda which previously duplicated across two migrations (and would also have been a third time in the tests I've added. So now both production sites and the new tests now share one implementation.

The "duplicate blocks at the top of the editor" symptom reported in the issue is a downstream TipTap render-time artefact of contentData entries that don't have a matching key in the markup. Once the keys in the markup are fixed, TipTap finds every match and stops inserting placeholders — no separate fix needed.

Fixes #22979

Testing

Automated

I've added new RteBlockHelperTests that verify this an existing functionality. The tests related to this fix fail against the original greedy regex and pass against the new one.

Manual

Spin up a site on 13, create an element type for a block and a document type for a page with a rich text editor. Configure the rich text editor to add the "Add Block" toolbar button and allow the element type as a block.

Create a content item and add two, sibling blocks to the RTE, and save.

Use that database to upgrade to 17.

You should see something like the following when you view the source of the RTE (which is what I got from my local test):

13:

<umb-rte-block class="ng-scope ng-isolate-scope" data-content-udi="umb://element/eae219cf7a04406e801b47542251a5c5"><!--Umbraco-Block--></umb-rte-block><umb-rte-block class="ng-scope ng-isolate-scope" data-content-udi="umb://element/dd352ff4cfd54ab68f5cc9a820bb3da6"><!--Umbraco-Block--></umb-rte-block>

Pre-fix upgrade to 17:

<umb-rte-block data-content-key="umb://element/eae219cf7a04406e801b47542251a5c5"></umb-rte-block><umb-rte-block
    data-content-key="dd352ff4-cfd5-4ab6-8f5c-c9a820bb3da6"></umb-rte-block>

Post-fix upgrade to 17:

<umb-rte-block data-content-key="eae219cf-7a04-406e-801b-47542251a5c5"></umb-rte-block><umb-rte-block
    data-content-key="dd352ff4-cfd5-4ab6-8f5c-c9a820bb3da6"></umb-rte-block>

[claude]

Claude finished @AndyButland's task in 5m 27s —— View job

---

PR Review

Target: origin/v17/dev · Based on commit: 5470bf11

Fixes a v13→v17 upgrade regression where greedy .* in the RTE block regex collapsed consecutive sibling <umb-rte-block> elements into one match, leaving all-but-last UDIs un-converted. The new non-greedy regex is correct, the shared helper DRYs up two identical implementations, and the 9 unit tests are thorough and verify the regression is caught.

• Modified public API: New public static partial class RteBlockHelper in Umbraco.Cms.Infrastructure.Migrations.Upgrade.Common with BlockRegex() and ConvertBlockUdisToKeys(string). Old public static partial class RteBlockHelper removed from Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_15_0_0.LocalLinks. ConvertRichTextEditorProperties changed from partial class to class (no binary impact).
• Affected implementations (outside this PR): None found — no other src/ files reference either RteBlockHelper.
• Breaking changes: The old RteBlockHelper class (in the LocalLinks namespace) is removed. It was public, carried [Obsolete("Scheduled for removal in Umbraco 18.")], and had a public static partial Regex BlockRegex() method. Removing it in v17 is a binary breaking change — external code compiled against the old assembly that called LocalLinks.RteBlockHelper.BlockRegex() will not compile against the new one.

---

Important

• src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/LocalLinks/LocalLinkRteProcessor.cs:79–94 (removed): The old RteBlockHelper in the LocalLinks namespace was a public class committed (via its [Obsolete] message) to remain until Umbraco 18. Removing it now in v17 is a binary breaking change per the repo policy. The practical risk is low given how deeply internal this type is, but the policy is clear. The safe fix is to keep a thin compatibility shim in the old namespace that delegates to the new one:

  // In LocalLinkRteProcessor.cs (bottom of file, or its own file in LocalLinks/)
  namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_15_0_0.LocalLinks;
  
  [Obsolete("Use Umbraco.Cms.Infrastructure.Migrations.Upgrade.Common.RteBlockHelper instead. Scheduled for removal in Umbraco 18.")]
  public static class RteBlockHelper
  {
      public static System.Text.RegularExpressions.Regex BlockRegex()
          => Common.RteBlockHelper.BlockRegex();
  }

• src/Umbraco.Infrastructure/Migrations/Upgrade/Common/RteBlockHelper.cs:9: The new class carries [Obsolete("Scheduled for removal in Umbraco 18.")], but the codebase policy (CLAUDE.md §5.4) requires new [Obsolete] attributes to target current_major + 2 = 19, not 18. The "matches its consumers" rationale is pragmatic but deviates from the rule. → Change to "Scheduled for removal in Umbraco 19." or explicitly document why 18 is chosen.

---

Suggestions

• src/Umbraco.Infrastructure/Migrations/Upgrade/Common/RteBlockHelper.cs:18: The inline comment on line 16–17 (// Non-greedy on both...) correctly explains the WHY — but it's on the regex line only. The ConvertBlockUdisToKeys method silently drops blocks whose UDI fails to parse (string.Empty on mismatch). This "drop on failure" behaviour is non-obvious and mirrors the original but isn't documented. A one-line note on the ConvertBlockUdisToKeys method explaining that malformed-UDI blocks are dropped (not preserved) would help future maintainers avoid inadvertently changing it.

• src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/LocalLinks/LocalLinkRteProcessor.cs:54: The comment // fix recursive hickup in ConvertRichTextEditorProperties contains a typo ("hickup" → "hiccup") and refers to an opaque mechanism. Since you're touching this line anyway, consider updating to something like // re-apply UDI→key conversion after local-link processing, in case ConvertRichTextEditorProperties ran before this processor — though this is strictly cosmetic.

---

Request Changes

The removal of the old LocalLinks.RteBlockHelper public class in v17 — before Umbraco 18 — violates the no-binary-breaking-changes policy. A one-line compatibility shim is all that's needed to make this clean. The [Obsolete] version on the new class is also worth correcting per policy. Everything else (the regex fix, the consolidation, the tests) is solid.

[Copilot]

Pull request overview

Fixes an upgrade migration bug (v13 → v17) where consecutive sibling <umb-rte-block> elements could be collapsed into a single regex match, leaving some data-content-udi values unconverted and causing downstream rendering artefacts.

Changes:

• Introduces RteBlockHelper to centralize the RTE block UDI→key conversion logic with a non-greedy, attribute-scoped regex.
• Updates both ConvertRichTextEditorProperties and LocalLinkRteProcessor to use the shared helper instead of duplicated regex code.
• Adds unit tests covering sibling-block scenarios and other edge cases.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/Upgrade/Common/RteBlockHelperTests.cs	Adds unit tests validating correct per-block matching and conversion behavior, including the reported sibling-block reproducer.
src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/LocalLinks/LocalLinkRteProcessor.cs	Replaces local duplicated conversion regex with the shared helper.
src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/ConvertRichTextEditorProperties.cs	Replaces migration’s duplicated conversion regex with the shared helper.
src/Umbraco.Infrastructure/Migrations/Upgrade/Common/RteBlockHelper.cs	Adds shared regex + conversion implementation intended for the v15 RTE block migration and related processing.

[Zeegaan]

Looks good as a preventive measure, but I'm also a little concerned with blocks that have already been migrated 🤔
Presumably you have to re-save all your blocks to get it working again 😅

[AndyButland]

Yes, this doesn't handle migrations that have already occurred - it's only resolving the situation for new migrations that would run into the same issue. I don't feel we can really justify a "just in case" migration across all blocks in a minor release to clean-up anyone who has already migrated and still has some blocks unconverted.

[AndyButland]

Cherry-picked into release/17.5.0. Not required for 18 as this migration has been removed.