Skip to content

Commit f233e6b

Browse files
committed
refactor(painter): delete dead fragment.X fallbacks from resolved-item reads (SD-2957)
The painter coalesced ~14 resolved-item field reads with the legacy fragment back-pointer (resolvedItem?.X ?? fragment.X). The resolve producer (resolveLayout) already copies these fields onto the resolved item whenever the source fragment has them, so the fragment fallback was mathematically equivalent to just resolvedItem?.X — dead code preserved from the SD-2836 transition. Sweep covers: * paragraph: continuesFromPrev, continuesOnNext, markerWidth (truthy use) * list-item content: continuesFromPrev, continuesOnNext, markerWidth (used as Math.max input — defaults to 0 when absent rather than reading fragment) * list-item wrapper frame: markerWidth (used as Math.max input — defaults to 0) * drop cap / paragraph / list marker / image: sourceAnchor * image: pmStart, pmEnd, metadata * fragment z-index: zIndex Test fixture updated: the list-item-wrapper geometry test was relying on the dead fallback (its resolved item omitted markerWidth). Populate markerWidth on both initial and updated resolved items so the test reflects the contract. Out of scope (different semantics, not the same dead-fallback shape): * block.width ?? fragment.width (image-block natural width vs fragment layout width — separate concepts). * fragment.lines ?? measure.lines.slice(...) — flagged as stretch. * fragment.scale ?? 1 — literal default with no resolved-stage equivalent.
1 parent 3fbf4b5 commit f233e6b

2 files changed

Lines changed: 23 additions & 27 deletions

File tree

packages/layout-engine/painters/dom/src/index.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5326,6 +5326,7 @@ describe('DomPainter', () => {
53265326
fragment: initialLayout.pages[0].fragments[0],
53275327
blockId: 'list-1',
53285328
fragmentIndex: 0,
5329+
markerWidth: 30,
53295330
block: listBlock as import('@superdoc/contracts').ListBlock,
53305331
measure: listMeasure as import('@superdoc/contracts').ListMeasure,
53315332
},
@@ -5358,6 +5359,7 @@ describe('DomPainter', () => {
53585359
fragment: updatedLayout.pages[0].fragments[0],
53595360
blockId: 'list-1',
53605361
fragmentIndex: 0,
5362+
markerWidth: 30,
53615363
block: listBlock as import('@superdoc/contracts').ListBlock,
53625364
measure: listMeasure as import('@superdoc/contracts').ListMeasure,
53635365
},

packages/layout-engine/painters/dom/src/renderer.ts

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3019,9 +3019,9 @@ export class DomPainter {
30193019
const content = resolvedItem?.content;
30203020

30213021
// Prefer resolved item metadata over legacy fragment reads
3022-
const paraContinuesFromPrev = resolvedItem?.continuesFromPrev ?? fragment.continuesFromPrev;
3023-
const paraContinuesOnNext = resolvedItem?.continuesOnNext ?? fragment.continuesOnNext;
3024-
const paraMarkerWidth = resolvedItem?.markerWidth ?? fragment.markerWidth;
3022+
const paraContinuesFromPrev = resolvedItem?.continuesFromPrev;
3023+
const paraContinuesOnNext = resolvedItem?.continuesOnNext;
3024+
const paraMarkerWidth = resolvedItem?.markerWidth;
30253025

30263026
const fragmentEl = this.doc.createElement('div');
30273027
fragmentEl.classList.add(CLASS_NAMES.fragment);
@@ -3181,10 +3181,7 @@ export class DomPainter {
31813181
const markerEl = this.doc!.createElement('span');
31823182
markerEl.classList.add('superdoc-paragraph-marker');
31833183
markerEl.textContent = resolvedMarker.text;
3184-
applySourceAnchorDataset(
3185-
markerEl,
3186-
resolvedMarker.sourceAnchor ?? resolvedItem?.sourceAnchor ?? fragment.sourceAnchor,
3187-
);
3184+
applySourceAnchorDataset(markerEl, resolvedMarker.sourceAnchor ?? resolvedItem?.sourceAnchor);
31883185
markerEl.style.pointerEvents = 'none';
31893186

31903187
markerContainer.style.position = 'relative';
@@ -3232,7 +3229,7 @@ export class DomPainter {
32323229
this.capturePaintSnapshotLine(lineEl, context, {
32333230
inTableFragment: false,
32343231
inTableParagraph: false,
3235-
sourceAnchor: resolvedItem?.sourceAnchor ?? fragment.sourceAnchor,
3232+
sourceAnchor: resolvedItem?.sourceAnchor,
32363233
});
32373234
fragmentEl.appendChild(lineEl);
32383235
});
@@ -3399,10 +3396,7 @@ export class DomPainter {
33993396
const markerEl = this.doc!.createElement('span');
34003397
markerEl.classList.add('superdoc-paragraph-marker');
34013398
markerEl.textContent = marker.markerText ?? '';
3402-
applySourceAnchorDataset(
3403-
markerEl,
3404-
block.sourceAnchor ?? resolvedItem?.sourceAnchor ?? fragment.sourceAnchor,
3405-
);
3399+
applySourceAnchorDataset(markerEl, block.sourceAnchor ?? resolvedItem?.sourceAnchor);
34063400
markerEl.style.pointerEvents = 'none';
34073401

34083402
const markerJustification = marker.justification ?? 'left';
@@ -3451,7 +3445,7 @@ export class DomPainter {
34513445
this.capturePaintSnapshotLine(lineEl, context, {
34523446
inTableFragment: false,
34533447
inTableParagraph: false,
3454-
sourceAnchor: resolvedItem?.sourceAnchor ?? fragment.sourceAnchor,
3448+
sourceAnchor: resolvedItem?.sourceAnchor,
34553449
});
34563450
fragmentEl.appendChild(lineEl);
34573451
});
@@ -3577,9 +3571,10 @@ export class DomPainter {
35773571
}
35783572

35793573
// Prefer resolved item metadata over legacy fragment reads
3580-
const listContinuesFromPrev = resolvedItem?.continuesFromPrev ?? fragment.continuesFromPrev;
3581-
const listContinuesOnNext = resolvedItem?.continuesOnNext ?? fragment.continuesOnNext;
3582-
const listMarkerWidth = resolvedItem?.markerWidth ?? fragment.markerWidth;
3574+
const listContinuesFromPrev = resolvedItem?.continuesFromPrev;
3575+
const listContinuesOnNext = resolvedItem?.continuesOnNext;
3576+
// Default to 0 (no marker gutter) when absent — used directly in Math.max below.
3577+
const listMarkerWidth = resolvedItem?.markerWidth ?? 0;
35833578

35843579
const fragmentEl = this.doc.createElement('div');
35853580
fragmentEl.classList.add(CLASS_NAMES.fragment, `${CLASS_NAMES.fragment}-list-item`);
@@ -3616,10 +3611,7 @@ export class DomPainter {
36163611

36173612
const markerEl = this.doc.createElement('span');
36183613
markerEl.classList.add('superdoc-list-marker');
3619-
applySourceAnchorDataset(
3620-
markerEl,
3621-
item.marker.sourceAnchor ?? item.sourceAnchor ?? resolvedItem?.sourceAnchor ?? fragment.sourceAnchor,
3622-
);
3614+
applySourceAnchorDataset(markerEl, item.marker.sourceAnchor ?? item.sourceAnchor ?? resolvedItem?.sourceAnchor);
36233615

36243616
// Track B: Use marker styling from wordLayout if available
36253617
const wordLayout: MinimalWordLayout | undefined = item.paragraph.attrs?.wordLayout as
@@ -3700,7 +3692,7 @@ export class DomPainter {
37003692
this.capturePaintSnapshotLine(lineEl, context, {
37013693
inTableFragment: false,
37023694
inTableParagraph: false,
3703-
sourceAnchor: resolvedItem?.sourceAnchor ?? fragment.sourceAnchor,
3695+
sourceAnchor: resolvedItem?.sourceAnchor,
37043696
});
37053697
contentEl.appendChild(lineEl);
37063698
});
@@ -3748,17 +3740,17 @@ export class DomPainter {
37483740
}
37493741

37503742
// Add PM position markers for transaction targeting
3751-
const imgPmStart = resolvedItem?.pmStart ?? fragment.pmStart;
3743+
const imgPmStart = resolvedItem?.pmStart;
37523744
if (imgPmStart != null) {
37533745
fragmentEl.dataset.pmStart = String(imgPmStart);
37543746
}
3755-
const imgPmEnd = resolvedItem?.pmEnd ?? fragment.pmEnd;
3747+
const imgPmEnd = resolvedItem?.pmEnd;
37563748
if (imgPmEnd != null) {
37573749
fragmentEl.dataset.pmEnd = String(imgPmEnd);
37583750
}
37593751

37603752
// Add metadata for interactive image resizing (skip watermarks - they should not be interactive)
3761-
const imgMetadata = resolvedItem?.metadata ?? fragment.metadata;
3753+
const imgMetadata = resolvedItem?.metadata;
37623754
if (imgMetadata && !block.attrs?.vmlWatermark) {
37633755
fragmentEl.setAttribute('data-image-metadata', JSON.stringify(imgMetadata));
37643756
}
@@ -6949,7 +6941,7 @@ export class DomPainter {
69496941
return '';
69506942
}
69516943

6952-
const zIndex = resolvedZIndex ?? fragment.zIndex;
6944+
const zIndex = resolvedZIndex;
69536945
return zIndex != null ? String(zIndex) : '';
69546946
}
69556947

@@ -6968,7 +6960,7 @@ export class DomPainter {
69686960
el.style.width = `${item.width}px`;
69696961
el.dataset.blockId = item.blockId;
69706962
el.dataset.layoutEpoch = String(this.layoutEpoch);
6971-
applySourceAnchorDataset(el, item.sourceAnchor ?? fragment.sourceAnchor);
6963+
applySourceAnchorDataset(el, item.sourceAnchor);
69726964
this.applyFragmentWrapperZIndex(el, fragment, item.zIndex);
69736965

69746966
if (item.fragmentKind === 'image' || item.fragmentKind === 'drawing' || item.fragmentKind === 'table') {
@@ -6992,7 +6984,9 @@ export class DomPainter {
69926984
section?: 'body' | 'header' | 'footer',
69936985
): void {
69946986
this.applyResolvedFragmentFrame(el, item, fragment, section);
6995-
const mw = item.markerWidth ?? fragment.markerWidth;
6987+
// Default to 0 (no marker gutter expansion) when markerWidth is absent — the resolve
6988+
// stage populates this for list items that have a measured marker (SD-2957).
6989+
const mw = item.markerWidth ?? 0;
69966990
el.style.left = `${item.x - mw}px`;
69976991
el.style.width = `${item.width + mw}px`;
69986992
}

0 commit comments

Comments
 (0)