Decoration range incorrectly expands to run boundaries; highlight can be lost after mark changes (#2173)

* fix: separate logic for decoration sync in presentation editor

* fix: move from overlay approach to paragraph splitting

* fix: clear selection

* fix: prevent highlighted decoration position shift

* fix: remove console.log

* fix: decoration restore condition

* fix(super-editor): compose decoration remap so external highlights track replacement and mark split

* fix: logic improvements

* fix: create shared helper for repeated code

* fix: update tests and address comments

* test: add behavior tests for decoration survival after mark changes

Playwright tests covering SD-1963: comment highlights and track-change
decorations must persist when the user applies bold/italic/underline to
overlapping or partially-overlapping text.

* fix(test): check comment highlight by id after run split

When italic is applied to part of a commented range, the run splits
and the highlight spans multiple DOM elements. Assert by commentId
and individual text fragments instead of the full contiguous text.

---------

Co-authored-by: Matthew Connelly <matthew@harbourshare.com>
Co-authored-by: Nick Bernal <nick@superdoc.dev>
Co-authored-by: Caio Pizzol <caio@harbourshare.com>

Author: 4 people

packages/layout-engine/layout-bridge/src/dom-reconciler.ts

@@ -383,7 +383,6 @@ export class DomReconciler {
 
     // Update position/size attributes if needed
     // (In a real implementation, this would update transform/position styles)
-
     return updated;
   }
 

packages/super-editor/src/core/presentation-editor/PresentationEditor.ts

@@ -110,6 +110,7 @@ import { ySyncPluginKey } from 'y-prosemirror';
 import type * as Y from 'yjs';
 import type { HeaderFooterDescriptor } from '../header-footer/HeaderFooterRegistry.js';
 import { isInRegisteredSurface } from './utils/uiSurfaceRegistry.js';
+import { splitRunsAtDecorationBoundaries } from './layout/SplitRunsAtDecorationBoundaries.js';
 
 // Types
 import type {
@@ -2341,8 +2342,10 @@ export class PresentationEditor extends EventEmitter {
   }
 
   /**
-   * Runs a full decoration bridge sync: reads external plugin decorations and
-   * reconciles them onto painted DOM elements (add/update/remove).
+   * Runs a full decoration sync: applies external plugin decoration classes
+   * and styles to the painted DOM elements via DecorationBridge. Runs are
+   * split at decoration boundaries during layout so only the selected portion
+   * gets the background (like the highlight mark, without applying a mark).
    *
    * Called synchronously from post-paint and observer-rebuild paths where the
    * DOM index is guaranteed to be fresh.
@@ -2354,7 +2357,9 @@ export class PresentationEditor extends EventEmitter {
     try {
       this.#decorationBridge.sync(state, this.#domPositionIndex);
     } catch (error) {
-      debugLog('warn', 'Decoration bridge sync failed', { error: String(error) });
+      // Sync can call findRangeByText and other doc-dependent logic; if it throws
+      // (e.g. edge-case doc state), avoid breaking the RAF or observer sync loop.
+      console.warn('[PresentationEditor] Decoration sync failed:', error);
     }
   }
 
@@ -2463,8 +2468,30 @@ export class PresentationEditor extends EventEmitter {
     // We listen on 'transaction' so the decoration bridge picks up changes
     // from any transaction type. The bridge's own identity check + RAF
     // coalescing prevent unnecessary work.
-    const handleTransaction = () => {
-      this.#scheduleDecorationSync();
+    // When decoration state changes without a doc change (e.g. setFocus), we must
+    // still run a full rerender so runs are split at the new decoration boundaries;
+    // otherwise the bridge applies the class to whole runs and highlights too much.
+    const handleTransaction = (event?: { transaction?: Transaction }) => {
+      const tr = event?.transaction;
+      this.#decorationBridge.recordTransaction(tr);
+      const state = this.#editor?.view?.state;
+      const decorationChanged = state && this.#decorationBridge.hasChanges(state);
+      // Sync immediately whenever decorations changed so e.g. clearFocus removes
+      // highlight-selection in the same tick. Only restore when we had a doc change.
+      if (decorationChanged) {
+        const restoreEmpty = tr ? tr.docChanged === true : false;
+        this.#decorationBridge.sync(state!, this.#domPositionIndex, {
+          restoreEmptyDecorations: restoreEmpty,
+        });
+      } else {
+        // No immediate sync; schedule coalesced sync on next frame.
+        this.#scheduleDecorationSync();
+      }
+      if (decorationChanged) {
+        this.#pendingDocChange = true;
+        this.#selectionSync.onLayoutStart();
+        this.#scheduleRerender();
+      }
     };
 
     this.#editor.on('update', handleUpdate);
@@ -3160,6 +3187,17 @@ export class PresentationEditor extends EventEmitter {
         return;
       }
 
+      // Split runs at decoration boundaries so bridge sync applies background only to the
+      // selected portion (like highlight mark) without adding a document mark.
+      const state = this.#editor?.view?.state;
+      const decorationRanges = state ? this.#decorationBridge.collectDecorationRanges(state) : [];
+      if (decorationRanges.length > 0) {
+        blocks = splitRunsAtDecorationBoundaries(
+          blocks,
+          decorationRanges.map((r) => ({ from: r.from, to: r.to })),
+        );
+      }
+
       this.#applyHtmlAnnotationMeasurements(blocks);
 
       const baseLayoutOptions = this.#resolveLayoutOptions(blocks, sectionMetadata);