Skip to content

Commit 9e6b0e7

Browse files
caio-pizzolharbournick
authored andcommitted
fix(painter-dom): honor appearance:hidden on inline SDTs (SD-3110) (superdoc-dev#3293)
* fix(painter-dom): honor appearance:hidden on inline SDTs (SD-3110) ECMA-376 §17.5.2.6 (w15:appearance val="hidden") means the SDT is present in the document for anchoring but visually transparent. Two prior leaks made hidden SDTs anything but: 1. The renderer painted full chrome (padding/border/hover/selected outline) regardless of appearance, leaving a visible bracket around the wrapped span. 2. The alias label was stamped into the DOM as a <span> child whose textContent included the alias. That text leaked into copy-paste (selecting and copying the wrapped phrase pulled in 'Inline content' / 'Harvey citation' / whatever the SDT's alias was) AND into screen-reader output. The data was already correct end-to-end on the converter side: import parses w15:appearance into the node attrs (extractAppearance in handle-structured-content-node.js), and the Document API surfaces it. The gap was that StructuredContentMetadata in @superdoc/contracts didn't carry the field, so the pm-adapter -> renderer bridge stripped it. Four-file fix: - contracts: add appearance?: StructuredContentAppearance to StructuredContentMetadata. - style-engine: read attrs.appearance in normalizeStructuredContentMetadata, validating against the three spec values (boundingBox | tags | hidden); unknown values are dropped rather than poisoning rendering. - painter-dom renderer: createInlineSdtWrapper now stamps data-appearance="hidden" on the wrapper AND skips appending the alias <span> entirely when hidden. - painter-dom styles: CSS rule keyed off [data-appearance='hidden'] zeroes padding/border/border-radius and neutralizes hover and selected states. Tests: - style-engine: appearance carries through, unknown values are dropped, omitted attr stays undefined. - painter-dom: render a hidden inline SDT and assert (a) data-appearance='hidden' is on the wrapper, (b) no .superdoc-structured-content-inline__label child exists, (c) wrapper.textContent equals exactly the wrapped phrase with the alias text nowhere in it. Verified: - @superdoc/painter-dom: 1071/1071 - @superdoc/style-engine: 132/132 - @superdoc/contracts: 232/232 - @superdoc/pm-adapter: 1838/1838 * test(behavior): cover hidden-appearance inline SDT (SD-3110) End-to-end coverage for the painter-dom fix in superdoc-dev#3293. Fixture is a 5-paragraph DOCX with inline SDTs across the appearance matrix: hidden, boundingBox, default (omitted), and two adjacent hidden. Five assertions, one per claim the PR makes plus a copy-paste smoke test: - data-appearance="hidden" stamped on hidden wrappers, absent on others - no __label child inside hidden wrappers; present on others - hidden wrappers omit the alias canary from textContent - no hidden-alias canary appears in the painted layout root - selection.toString() over a hidden wrapper returns only the wrapped phrase Visual coverage follow-up: drop a slim variant in tests/visual/test-data/ via pnpm docs:upload (corpus is R2-backed, not in-tree). * fix(painter-dom): keep hidden inline SDTs out of lock-hover styling (SD-3110) Caught in review: the lock-hover rule .superdoc-structured-content-inline[data-lock-mode]:hover:not(.ProseMirror-selectednode) has (0,4,0) specificity, one higher than the hidden-appearance rule's (0,3,0). Hovering a hidden inline SDT therefore re-introduced the blue background and z-index 9999999 boost the rule meant to suppress. Exclude data-appearance="hidden" from the inline branch of the lock-hover rule. Block-level branch is untouched; block hidden isn't a render path yet. Adds a behavior regression assertion: hover a hidden wrapper and verify the computed backgroundColor doesn't pick up the lock-hover blue and z-index doesn't jump to 9999999. 18/18 behavior cases × 3 browsers green; painter-dom unit tests still 1071/1071. * fix(painter-dom): restore viewing-mode hover suppression on inline SDTs (SD-3110) The previous fix added a second chained :not([data-appearance='hidden']) to the inline lock-hover rule, which bumped its specificity from (0,4,0) to (0,5,0). The viewing-mode suppression rule below sits at (0,4,0), so it lost the cascade and the lock-hover blue re-appeared on hover in viewing mode — regressing the SD-2232 behavior test "inline SDT hover does not show background in viewing mode". Collapse the two predicates into a single :not(a, b). Comma-list :not() takes the max specificity of its arguments, not the sum, so the selector stays at (0,4,0), viewing-mode suppression wins again, and the hidden-appearance exclusion is preserved. Verified: 22/22 SDT behavior cases on chromium, 44/44 on firefox+webkit; painter-dom unit tests still 1071/1071.
1 parent 15ab9c8 commit 9e6b0e7

8 files changed

Lines changed: 315 additions & 3 deletions

File tree

packages/layout-engine/contracts/src/index.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,27 @@ export type FieldAnnotationMetadata = {
121121

122122
export type StructuredContentLockMode = 'unlocked' | 'sdtLocked' | 'contentLocked' | 'sdtContentLocked';
123123

124+
/**
125+
* Visual chrome / labelling behavior of an SDT, mirroring
126+
* `<w15:appearance w15:val="…">` (ECMA-376 §17.5.2.6 / OOXML 2010+).
127+
*
128+
* - `'boundingBox'` (default): visible chrome around the SDT content.
129+
* - `'tags'`: tags-only mode (start/end markers).
130+
* - `'hidden'`: no chrome at all; the SDT exists in the document but is
131+
* visually transparent. The alias label MUST NOT leak into the rendered
132+
* DOM textContent (a11y / copy-paste behavior).
133+
*/
134+
export type StructuredContentAppearance = 'boundingBox' | 'tags' | 'hidden';
135+
124136
export type StructuredContentMetadata = {
125137
type: 'structuredContent';
126138
scope: 'inline' | 'block';
127139
id?: string | null;
128140
tag?: string | null;
129141
alias?: string | null;
130142
lockMode?: StructuredContentLockMode;
143+
/** Appearance from the SDT's `<w15:appearance>` element, when present. */
144+
appearance?: StructuredContentAppearance;
131145
sdtPr?: unknown;
132146
};
133147

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2694,6 +2694,87 @@ describe('DomPainter', () => {
26942694
expect(wrapper.textContent).toContain('controlled text');
26952695
});
26962696

2697+
it('omits chrome and alias label when inline SDT appearance is hidden (SD-3110)', () => {
2698+
// ECMA-376 `<w15:appearance w15:val="hidden"/>` should render the
2699+
// SDT transparently: no padding/border/label, and the alias text
2700+
// MUST NOT appear in DOM textContent (copy-paste / screen reader
2701+
// leak otherwise).
2702+
const block: FlowBlock = {
2703+
kind: 'paragraph',
2704+
id: 'inline-sc-hidden',
2705+
runs: [
2706+
{ text: 'See ', fontFamily: 'Arial', fontSize: 16, pmStart: 0, pmEnd: 4 },
2707+
{
2708+
text: 'Alpha Corp v. SEC',
2709+
fontFamily: 'Arial',
2710+
fontSize: 16,
2711+
pmStart: 4,
2712+
pmEnd: 21,
2713+
sdt: {
2714+
type: 'structuredContent',
2715+
scope: 'inline',
2716+
id: 'sc-hidden-1',
2717+
tag: 'citation',
2718+
alias: 'Harvey citation',
2719+
appearance: 'hidden',
2720+
},
2721+
},
2722+
{ text: ' today.', fontFamily: 'Arial', fontSize: 16, pmStart: 21, pmEnd: 28 },
2723+
],
2724+
attrs: {},
2725+
};
2726+
2727+
const measure: Measure = {
2728+
kind: 'paragraph',
2729+
lines: [
2730+
{ fromRun: 0, fromChar: 0, toRun: 2, toChar: 7, width: 200, ascent: 12, descent: 4, lineHeight: 20 },
2731+
],
2732+
totalHeight: 20,
2733+
};
2734+
2735+
const layout: Layout = {
2736+
pageSize: { w: 612, h: 792 },
2737+
pages: [
2738+
{
2739+
number: 1,
2740+
fragments: [
2741+
{
2742+
kind: 'para',
2743+
blockId: 'inline-sc-hidden',
2744+
fromLine: 0,
2745+
toLine: 1,
2746+
x: 30,
2747+
y: 40,
2748+
width: 552,
2749+
pmStart: 0,
2750+
pmEnd: 28,
2751+
},
2752+
],
2753+
},
2754+
],
2755+
};
2756+
2757+
const painter = createTestPainter({ blocks: [block], measures: [measure] });
2758+
painter.paint(layout, mount);
2759+
2760+
const wrapper = mount.querySelector(
2761+
'.superdoc-structured-content-inline[data-sdt-id="sc-hidden-1"]',
2762+
) as HTMLElement | null;
2763+
expect(wrapper).toBeTruthy();
2764+
if (!wrapper) return;
2765+
2766+
// data-appearance="hidden" is the hook CSS uses to drop chrome.
2767+
expect(wrapper.dataset.appearance).toBe('hidden');
2768+
2769+
// No alias label child — must not be in the DOM at all.
2770+
expect(wrapper.querySelector('.superdoc-structured-content-inline__label')).toBeNull();
2771+
2772+
// textContent of the wrapper must equal exactly the wrapped phrase,
2773+
// with no alias text leaked in.
2774+
expect(wrapper.textContent).toBe('Alpha Corp v. SEC');
2775+
expect(wrapper.textContent).not.toContain('Harvey citation');
2776+
});
2777+
26972778
it('keeps inline SDT wrapper font-size in sync when run font-size changes', () => {
26982779
const block: FlowBlock = {
26992780
kind: 'paragraph',

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7308,12 +7308,29 @@ export class DomPainter {
73087308
/**
73097309
* Create an inline SDT wrapper `<span>` with className, layoutEpoch, dataset, and label.
73107310
* Shared by both the geometry and run-based rendering paths.
7311+
*
7312+
* When the SDT's `appearance` is `'hidden'` (matching ECMA-376
7313+
* `<w15:appearance w15:val="hidden"/>`), the wrapper is rendered
7314+
* transparently: chrome is suppressed via `data-appearance="hidden"`
7315+
* (see styles.ts) and the alias label is omitted entirely. Without the
7316+
* latter, the alias text leaks into the rendered DOM `textContent`
7317+
* (copy-paste includes it) and screen readers announce it.
73117318
*/
73127319
private createInlineSdtWrapper(sdt: SdtMetadata): HTMLElement {
73137320
const wrapper = this.doc!.createElement('span');
73147321
wrapper.className = DOM_CLASS_NAMES.INLINE_SDT_WRAPPER;
73157322
wrapper.dataset.layoutEpoch = String(this.layoutEpoch);
73167323
this.applySdtDataset(wrapper, sdt);
7324+
7325+
const appearance =
7326+
sdt.type === 'structuredContent' ? (sdt as { appearance?: string }).appearance : undefined;
7327+
if (appearance === 'hidden') {
7328+
wrapper.dataset.appearance = 'hidden';
7329+
// No alias label and no chrome: see CSS rule keyed off
7330+
// `[data-appearance="hidden"]`.
7331+
return wrapper;
7332+
}
7333+
73177334
const alias = (sdt as { alias?: string })?.alias || 'Inline content';
73187335
const labelEl = this.doc!.createElement('span');
73197336
labelEl.className = `${DOM_CLASS_NAMES.INLINE_SDT_WRAPPER}__label`;

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

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -642,13 +642,38 @@ const SDT_CONTAINER_STYLES = `
642642
display: none;
643643
}
644644
645+
/* Hidden appearance per ECMA-376 (w15:appearance val="hidden"). SDT
646+
* exists in the document for anchoring but is visually transparent: no
647+
* padding, no border, no hover background, no selected outline. The
648+
* alias label is not emitted into the DOM at all (see renderer.ts), so
649+
* there is nothing to hide from copy-paste or screen readers. */
650+
.superdoc-structured-content-inline[data-appearance='hidden'] {
651+
padding: 0;
652+
border: none;
653+
border-radius: 0;
654+
}
655+
.superdoc-structured-content-inline[data-appearance='hidden']:hover {
656+
background-color: transparent;
657+
border: none;
658+
}
659+
.superdoc-structured-content-inline[data-appearance='hidden'].ProseMirror-selectednode {
660+
border-color: transparent;
661+
background-color: transparent;
662+
}
663+
645664
/* Hover highlight for SDT containers.
646665
* Hover adds background highlight and z-index boost.
647666
* Block SDTs use .sdt-group-hover class (event delegation for multi-fragment coordination).
648667
* Inline SDTs use :hover (single element, no coordination needed).
649-
* Hover is suppressed when the node is selected (SD-1584). */
668+
* Hover is suppressed when the node is selected (SD-1584).
669+
*
670+
* Inline SDTs with appearance=hidden are excluded via the same :not()
671+
* that handles selection. Both predicates live in one :not(a, b) so the
672+
* selector keeps (0,4,0) specificity. A second chained :not() would push
673+
* it to (0,5,0) and beat the viewing-mode suppression rule below, which
674+
* also sits at (0,4,0). */
650675
.superdoc-structured-content-block[data-lock-mode].sdt-group-hover:not(.ProseMirror-selectednode),
651-
.superdoc-structured-content-inline[data-lock-mode]:hover:not(.ProseMirror-selectednode) {
676+
.superdoc-structured-content-inline[data-lock-mode]:hover:not(.ProseMirror-selectednode, [data-appearance='hidden']) {
652677
background-color: var(--sd-content-controls-lock-hover-bg, rgba(98, 155, 231, 0.08));
653678
z-index: 9999999;
654679
}

packages/layout-engine/style-engine/src/index.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,35 @@ describe('resolveSdtMetadata', () => {
8787
});
8888
});
8989

90+
it('carries appearance through for inline structured content (SD-3110)', () => {
91+
const metadata = resolveSdtMetadata({
92+
nodeType: 'structuredContent',
93+
attrs: { id: '7', tag: 'citation', alias: 'Harvey citation', appearance: 'hidden' },
94+
});
95+
expect(metadata).toMatchObject({
96+
type: 'structuredContent',
97+
scope: 'inline',
98+
appearance: 'hidden',
99+
});
100+
});
101+
102+
it('drops unknown appearance values rather than letting them flow to the renderer', () => {
103+
const metadata = resolveSdtMetadata({
104+
nodeType: 'structuredContent',
105+
attrs: { id: '8', tag: 'x', appearance: 'malformed' },
106+
});
107+
expect(metadata).toMatchObject({ type: 'structuredContent', scope: 'inline' });
108+
expect((metadata as { appearance?: string }).appearance).toBeUndefined();
109+
});
110+
111+
it('omits appearance when the source attr is missing', () => {
112+
const metadata = resolveSdtMetadata({
113+
nodeType: 'structuredContent',
114+
attrs: { id: '9', tag: 'x' },
115+
});
116+
expect((metadata as { appearance?: string }).appearance).toBeUndefined();
117+
});
118+
90119
it('normalizes document section metadata', () => {
91120
const metadata = resolveSdtMetadata({
92121
nodeType: 'documentSection',

packages/layout-engine/style-engine/src/index.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ function normalizeStructuredContentMetadata(
241241
nodeType: 'structuredContent' | 'structuredContentBlock',
242242
attrs: Record<string, unknown>,
243243
): StructuredContentMetadata {
244-
return {
244+
const metadata: StructuredContentMetadata = {
245245
type: 'structuredContent',
246246
scope: nodeType === 'structuredContentBlock' ? 'block' : 'inline',
247247
id: toNullableString(attrs.id),
@@ -250,6 +250,14 @@ function normalizeStructuredContentMetadata(
250250
lockMode: attrs.lockMode as StructuredContentMetadata['lockMode'],
251251
sdtPr: attrs.sdtPr,
252252
};
253+
// `appearance` comes from the SDT's <w15:appearance> element on import.
254+
// Only the three spec-defined values flow through; anything else is
255+
// discarded so a bad value doesn't poison rendering decisions.
256+
const rawAppearance = toOptionalString(attrs.appearance);
257+
if (rawAppearance === 'boundingBox' || rawAppearance === 'tags' || rawAppearance === 'hidden') {
258+
metadata.appearance = rawAppearance;
259+
}
260+
return metadata;
253261
}
254262

255263
function normalizeDocumentSectionMetadata(attrs: Record<string, unknown>): DocumentSectionMetadata {
Binary file not shown.
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import { fileURLToPath } from 'node:url';
2+
import path from 'node:path';
3+
import { test, expect } from '../../fixtures/superdoc.js';
4+
5+
const __dirname = path.dirname(fileURLToPath(import.meta.url));
6+
const DOC_PATH = path.resolve(__dirname, 'fixtures/sd-3110-inline-sdt-appearance-variants.docx');
7+
8+
test.use({ config: { toolbar: 'full', showSelection: true } });
9+
10+
// The fixture has five paragraphs; we keep the wrapper-by-sdtId mapping
11+
// explicit because it's the contract this spec asserts against.
12+
const HIDDEN_IDS = ['1001', '1004', '1005'] as const;
13+
const VISIBLE_IDS = ['1002', '1003'] as const; // boundingBox + omitted (default)
14+
const HIDDEN_ALIAS_CANARIES = [
15+
'HIDDEN_ALIAS_LEAK_CANARY',
16+
'HIDDEN_ALIAS_DOUBLE_A',
17+
'HIDDEN_ALIAS_DOUBLE_B',
18+
] as const;
19+
20+
const INLINE_SDT = '.superdoc-structured-content-inline';
21+
const INLINE_LABEL = '.superdoc-structured-content-inline__label';
22+
23+
test.describe('inline SDT appearance=hidden (SD-3110)', () => {
24+
test.beforeEach(async ({ superdoc }) => {
25+
await superdoc.loadDocument(DOC_PATH);
26+
await superdoc.waitForStable();
27+
});
28+
29+
test('hidden wrappers carry data-appearance="hidden" and visible ones do not', async ({ superdoc }) => {
30+
const attrs = await superdoc.page.evaluate((sel) => {
31+
return Array.from(document.querySelectorAll(sel)).map((el) => ({
32+
sdtId: (el as HTMLElement).dataset.sdtId ?? null,
33+
appearance: (el as HTMLElement).dataset.appearance ?? null,
34+
}));
35+
}, INLINE_SDT);
36+
37+
const byId = new Map(attrs.map((a) => [a.sdtId, a.appearance]));
38+
for (const id of HIDDEN_IDS) expect(byId.get(id)).toBe('hidden');
39+
for (const id of VISIBLE_IDS) expect(byId.get(id)).toBeNull();
40+
});
41+
42+
test('hidden wrappers have no alias label child; visible wrappers do', async ({ superdoc }) => {
43+
const labelPresence = await superdoc.page.evaluate(
44+
({ sel, labelSel }) => {
45+
return Array.from(document.querySelectorAll(sel)).map((el) => ({
46+
sdtId: (el as HTMLElement).dataset.sdtId ?? null,
47+
hasLabel: !!el.querySelector(labelSel),
48+
}));
49+
},
50+
{ sel: INLINE_SDT, labelSel: INLINE_LABEL },
51+
);
52+
53+
const byId = new Map(labelPresence.map((a) => [a.sdtId, a.hasLabel]));
54+
for (const id of HIDDEN_IDS) expect(byId.get(id)).toBe(false);
55+
for (const id of VISIBLE_IDS) expect(byId.get(id)).toBe(true);
56+
});
57+
58+
test('hidden wrappers omit the alias canary from textContent', async ({ superdoc }) => {
59+
const textByIdRaw = await superdoc.page.evaluate((sel) => {
60+
return Array.from(document.querySelectorAll(sel)).map((el) => ({
61+
sdtId: (el as HTMLElement).dataset.sdtId ?? null,
62+
text: el.textContent ?? '',
63+
}));
64+
}, INLINE_SDT);
65+
const textById = new Map(textByIdRaw.map((a) => [a.sdtId, a.text]));
66+
67+
expect(textById.get('1001')).toBe('Alpha Corp v. SEC');
68+
expect(textById.get('1004')).toBe('first hidden span');
69+
expect(textById.get('1005')).toBe('second hidden span');
70+
71+
// Visible wrappers still surface the alias as a label — that's the
72+
// pre-existing boundingBox/default behavior.
73+
expect(textById.get('1002')).toContain('VISIBLE_ALIAS_FOR_COMPARISON');
74+
expect(textById.get('1003')).toContain('DEFAULT_APPEARANCE_ALIAS');
75+
});
76+
77+
test('no hidden-SDT alias canary appears anywhere in the painted layout', async ({ superdoc }) => {
78+
const layoutText = await superdoc.page.evaluate(() => {
79+
// .presentation-editor__pages is the painter-dom root; selection,
80+
// copy, and visual reads operate on it.
81+
const root =
82+
document.querySelector('.presentation-editor__pages') ??
83+
document.querySelector('.superdoc-layout');
84+
return root?.textContent ?? '';
85+
});
86+
87+
for (const canary of HIDDEN_ALIAS_CANARIES) {
88+
expect(layoutText).not.toContain(canary);
89+
}
90+
});
91+
92+
test('hovering a hidden wrapper does not paint the lock-hover background or boost z-index', async ({ superdoc }) => {
93+
// Regression guard for the CSS specificity bug caught in PR review:
94+
// the lock-hover rule
95+
// .superdoc-structured-content-inline[data-lock-mode]:hover:not(.ProseMirror-selectednode)
96+
// has (0,4,0) specificity vs (0,3,0) for the hidden-appearance hover
97+
// rule, so without an explicit :not([data-appearance='hidden']) it
98+
// re-introduces the lock-hover blue background + z-index 9999999 on
99+
// hover, contradicting "visually transparent".
100+
// Painter may emit more than one wrapper for the same SDT when the run
101+
// is split across lines/fragments — each fragment carries the same
102+
// data-sdt-id. Scope to the painter class and take `.first()`: the
103+
// CSS specificity bug is per-element, so a single wrapper is enough.
104+
const wrapper = superdoc.page
105+
.locator('.superdoc-structured-content-inline[data-sdt-id="1001"]')
106+
.first();
107+
await wrapper.hover();
108+
await superdoc.waitForStable();
109+
110+
const styles = await wrapper.evaluate((el) => {
111+
const cs = getComputedStyle(el);
112+
return { backgroundColor: cs.backgroundColor, zIndex: cs.zIndex };
113+
});
114+
115+
// Default backgrounds on most browsers are transparent / rgba(0, 0, 0, 0);
116+
// the regression value is rgba(98, 155, 231, 0.08).
117+
expect(styles.backgroundColor).not.toContain('98, 155, 231');
118+
// The lock-hover rule sets z-index 9999999 on top of any default — if
119+
// it slipped through, the hidden wrapper would jump above siblings.
120+
expect(styles.zIndex).not.toBe('9999999');
121+
});
122+
123+
test('selecting a hidden wrapper copies only the wrapped phrase', async ({ superdoc }) => {
124+
const selectionText = await superdoc.page.evaluate(() => {
125+
const wrapper = document.querySelector('[data-sdt-id="1001"]');
126+
if (!wrapper) return null;
127+
const range = document.createRange();
128+
range.selectNodeContents(wrapper);
129+
const sel = window.getSelection();
130+
sel?.removeAllRanges();
131+
sel?.addRange(range);
132+
return sel?.toString() ?? null;
133+
});
134+
135+
expect(selectionText).toBe('Alpha Corp v. SEC');
136+
expect(selectionText).not.toContain('HIDDEN_ALIAS_LEAK_CANARY');
137+
});
138+
});

0 commit comments

Comments
 (0)