From 92c13e1a0c0c04256bf738eec3871d70e0d0d515 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Sat, 28 Mar 2026 11:42:48 -0700 Subject: [PATCH 1/3] fix: normalize footnote reference markers before superscript layout --- .../endnote-reference.test.ts | 97 +++++++++ .../inline-converters/endnote-reference.ts | 36 +-- .../footnote-reference.test.ts | 97 +++++++++ .../inline-converters/footnote-reference.ts | 41 +--- .../reference-marker.test.ts | 206 ++++++++++++++++++ .../inline-converters/reference-marker.ts | 140 ++++++++++++ 6 files changed, 546 insertions(+), 71 deletions(-) create mode 100644 packages/layout-engine/pm-adapter/src/converters/inline-converters/endnote-reference.test.ts create mode 100644 packages/layout-engine/pm-adapter/src/converters/inline-converters/footnote-reference.test.ts create mode 100644 packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.test.ts create mode 100644 packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.ts diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/endnote-reference.test.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/endnote-reference.test.ts new file mode 100644 index 0000000000..2171fcea54 --- /dev/null +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/endnote-reference.test.ts @@ -0,0 +1,97 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { TextRun } from '@superdoc/contracts'; +import type { PMNode } from '../../types.js'; +import type { InlineConverterParams } from './common.js'; +import { SUBSCRIPT_SUPERSCRIPT_SCALE } from '../../constants.js'; + +vi.mock('./text-run.js', () => ({ + textNodeToRun: vi.fn( + (params: InlineConverterParams): TextRun => ({ + text: params.node.text || '', + fontFamily: params.defaultFont, + fontSize: params.defaultSize, + }), + ), +})); + +import { endnoteReferenceToBlock } from './endnote-reference.js'; + +function makeParams(overrides: Partial = {}): InlineConverterParams { + const node: PMNode = { type: 'endnoteReference', attrs: { id: '1' } }; + return { + node, + positions: new WeakMap(), + defaultFont: 'Calibri', + defaultSize: 16, + inheritedMarks: [], + sdtMetadata: undefined, + hyperlinkConfig: { enableRichHyperlinks: false }, + themeColors: undefined, + runProperties: undefined, + paragraphProperties: undefined, + converterContext: { + endnoteNumberById: { '1': 1, '2': 2, '10': 10 }, + } as unknown as InlineConverterParams['converterContext'], + enableComments: false, + visitNode: vi.fn(), + bookmarks: undefined, + tabOrdinal: 0, + paragraphAttrs: {}, + nextBlockId: vi.fn(), + ...overrides, + } as InlineConverterParams; +} + +describe('endnoteReferenceToBlock', () => { + it('emits plain digit text for an endnote marker', () => { + const run = endnoteReferenceToBlock(makeParams()); + + expect(run.text).toBe('1'); + }); + + it('does not emit Unicode superscript glyphs', () => { + const run = endnoteReferenceToBlock(makeParams()); + + expect(run.text).not.toMatch(/[⁰¹²³⁴⁵⁶⁷⁸⁹]/); + }); + + it('resolves the display number from endnoteNumberById', () => { + const node: PMNode = { type: 'endnoteReference', attrs: { id: '2' } }; + const run = endnoteReferenceToBlock(makeParams({ node })); + + expect(run.text).toBe('2'); + }); + + it('resolves multi-digit display numbers', () => { + const node: PMNode = { type: 'endnoteReference', attrs: { id: '10' } }; + const run = endnoteReferenceToBlock(makeParams({ node })); + + expect(run.text).toBe('10'); + }); + + it('falls back to raw id when endnoteNumberById has no mapping', () => { + const node: PMNode = { type: 'endnoteReference', attrs: { id: '99' } }; + const run = endnoteReferenceToBlock(makeParams({ node })); + + expect(run.text).toBe('99'); + }); + + it('falls back to asterisk when id is missing', () => { + const node: PMNode = { type: 'endnoteReference', attrs: {} }; + const run = endnoteReferenceToBlock(makeParams({ node })); + + expect(run.text).toBe('*'); + }); + + it('sets vertAlign to superscript', () => { + const run = endnoteReferenceToBlock(makeParams()); + + expect(run.vertAlign).toBe('superscript'); + }); + + it('scales fontSize from the paragraph base', () => { + const run = endnoteReferenceToBlock(makeParams({ defaultSize: 16 })); + + expect(run.fontSize).toBe(16 * SUBSCRIPT_SUPERSCRIPT_SCALE); + }); +}); diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/endnote-reference.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/endnote-reference.ts index 3619142b73..09cc97eeef 100644 --- a/packages/layout-engine/pm-adapter/src/converters/inline-converters/endnote-reference.ts +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/endnote-reference.ts @@ -1,26 +1,13 @@ import type { TextRun } from '@superdoc/contracts'; -import type { PMNode } from '../../types.js'; -import { textNodeToRun } from './text-run.js'; +import { buildReferenceMarkerRun } from './reference-marker.js'; import type { InlineConverterParams } from './common.js'; export function endnoteReferenceToBlock(params: InlineConverterParams): TextRun { const { node, converterContext } = params; - const refPos = params.positions.get(node); const id = (node.attrs as Record | undefined)?.id; const displayId = resolveEndnoteDisplayNumber(id, converterContext.endnoteNumberById) ?? id ?? '*'; - const displayText = toSuperscriptDigits(displayId); - const run = textNodeToRun({ - ...params, - node: { type: 'text', text: displayText, marks: [...(node.marks ?? [])] } as PMNode, - }); - - if (refPos) { - run.pmStart = refPos.start; - run.pmEnd = refPos.end; - } - - return run; + return buildReferenceMarkerRun(String(displayId), params); } const resolveEndnoteDisplayNumber = (id: unknown, endnoteNumberById: Record | undefined): unknown => { @@ -29,22 +16,3 @@ const resolveEndnoteDisplayNumber = (id: unknown, endnoteNumberById: Record 0 ? mapped : null; }; - -const toSuperscriptDigits = (value: unknown): string => { - const map: Record = { - '0': '⁰', - '1': '¹', - '2': '²', - '3': '³', - '4': '⁴', - '5': '⁵', - '6': '⁶', - '7': '⁷', - '8': '⁸', - '9': '⁹', - }; - return String(value ?? '') - .split('') - .map((ch) => map[ch] ?? ch) - .join(''); -}; diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/footnote-reference.test.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/footnote-reference.test.ts new file mode 100644 index 0000000000..07bc1a6500 --- /dev/null +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/footnote-reference.test.ts @@ -0,0 +1,97 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { TextRun } from '@superdoc/contracts'; +import type { PMNode, PositionMap } from '../../types.js'; +import type { InlineConverterParams } from './common.js'; +import { SUBSCRIPT_SUPERSCRIPT_SCALE } from '../../constants.js'; + +vi.mock('./text-run.js', () => ({ + textNodeToRun: vi.fn( + (params: InlineConverterParams): TextRun => ({ + text: params.node.text || '', + fontFamily: params.defaultFont, + fontSize: params.defaultSize, + }), + ), +})); + +import { footnoteReferenceToBlock } from './footnote-reference.js'; + +function makeParams(overrides: Partial = {}): InlineConverterParams { + const node: PMNode = { type: 'footnoteReference', attrs: { id: '1' } }; + return { + node, + positions: new WeakMap(), + defaultFont: 'Calibri', + defaultSize: 16, + inheritedMarks: [], + sdtMetadata: undefined, + hyperlinkConfig: { enableRichHyperlinks: false }, + themeColors: undefined, + runProperties: undefined, + paragraphProperties: undefined, + converterContext: { + footnoteNumberById: { '1': 1, '2': 2, '10': 10 }, + } as unknown as InlineConverterParams['converterContext'], + enableComments: false, + visitNode: vi.fn(), + bookmarks: undefined, + tabOrdinal: 0, + paragraphAttrs: {}, + nextBlockId: vi.fn(), + ...overrides, + } as InlineConverterParams; +} + +describe('footnoteReferenceToBlock', () => { + it('emits plain digit text for a footnote marker', () => { + const run = footnoteReferenceToBlock(makeParams()); + + expect(run.text).toBe('1'); + }); + + it('does not emit Unicode superscript glyphs', () => { + const run = footnoteReferenceToBlock(makeParams()); + + expect(run.text).not.toMatch(/[⁰¹²³⁴⁵⁶⁷⁸⁹]/); + }); + + it('resolves the display number from footnoteNumberById', () => { + const node: PMNode = { type: 'footnoteReference', attrs: { id: '2' } }; + const run = footnoteReferenceToBlock(makeParams({ node })); + + expect(run.text).toBe('2'); + }); + + it('resolves multi-digit display numbers', () => { + const node: PMNode = { type: 'footnoteReference', attrs: { id: '10' } }; + const run = footnoteReferenceToBlock(makeParams({ node })); + + expect(run.text).toBe('10'); + }); + + it('falls back to raw id when footnoteNumberById has no mapping', () => { + const node: PMNode = { type: 'footnoteReference', attrs: { id: '99' } }; + const run = footnoteReferenceToBlock(makeParams({ node })); + + expect(run.text).toBe('99'); + }); + + it('falls back to asterisk when id is missing', () => { + const node: PMNode = { type: 'footnoteReference', attrs: {} }; + const run = footnoteReferenceToBlock(makeParams({ node })); + + expect(run.text).toBe('*'); + }); + + it('sets vertAlign to superscript', () => { + const run = footnoteReferenceToBlock(makeParams()); + + expect(run.vertAlign).toBe('superscript'); + }); + + it('scales fontSize from the paragraph base', () => { + const run = footnoteReferenceToBlock(makeParams({ defaultSize: 16 })); + + expect(run.fontSize).toBe(16 * SUBSCRIPT_SUPERSCRIPT_SCALE); + }); +}); diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/footnote-reference.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/footnote-reference.ts index a5113d8215..f7810dff0a 100644 --- a/packages/layout-engine/pm-adapter/src/converters/inline-converters/footnote-reference.ts +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/footnote-reference.ts @@ -1,27 +1,13 @@ -import { TextRun } from '@superdoc/contracts'; -import { PMNode } from '../../types'; -import { textNodeToRun } from './text-run'; -import type { InlineConverterParams } from './common'; +import type { TextRun } from '@superdoc/contracts'; +import { buildReferenceMarkerRun } from './reference-marker.js'; +import type { InlineConverterParams } from './common.js'; export function footnoteReferenceToBlock(params: InlineConverterParams): TextRun { const { node, converterContext } = params; - const refPos = params.positions.get(node); const id = (node.attrs as Record | undefined)?.id; const displayId = resolveFootnoteDisplayNumber(id, converterContext.footnoteNumberById) ?? id ?? '*'; - const displayText = toSuperscriptDigits(displayId); - const run = textNodeToRun({ - ...params, - node: { type: 'text', text: displayText, marks: [...(node.marks ?? [])] } as PMNode, - }); - - // Copy PM positions from the parent footnoteReference node - if (refPos) { - run.pmStart = refPos.start; - run.pmEnd = refPos.end; - } - - return run; + return buildReferenceMarkerRun(String(displayId), params); } const resolveFootnoteDisplayNumber = (id: unknown, footnoteNumberById: Record | undefined): unknown => { @@ -30,22 +16,3 @@ const resolveFootnoteDisplayNumber = (id: unknown, footnoteNumberById: Record 0 ? mapped : null; }; - -const toSuperscriptDigits = (value: unknown): string => { - const map: Record = { - '0': '⁰', - '1': '¹', - '2': '²', - '3': '³', - '4': '⁴', - '5': '⁵', - '6': '⁶', - '7': '⁷', - '8': '⁸', - '9': '⁹', - }; - return String(value ?? '') - .split('') - .map((ch) => map[ch] ?? ch) - .join(''); -}; diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.test.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.test.ts new file mode 100644 index 0000000000..9867dfaa4b --- /dev/null +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.test.ts @@ -0,0 +1,206 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { TextRun } from '@superdoc/contracts'; +import type { PMMark, PMNode, PositionMap } from '../../types.js'; +import type { InlineConverterParams } from './common.js'; +import { SUBSCRIPT_SUPERSCRIPT_SCALE } from '../../constants.js'; + +vi.mock('./text-run.js', () => ({ + textNodeToRun: vi.fn( + (params: InlineConverterParams): TextRun => ({ + text: params.node.text || '', + fontFamily: params.defaultFont, + fontSize: params.defaultSize, + }), + ), +})); + +import { buildReferenceMarkerRun } from './reference-marker.js'; +import { textNodeToRun } from './text-run.js'; + +const DEFAULT_FONT_SIZE = 16; + +function makeParams(overrides: Partial = {}): InlineConverterParams { + const node: PMNode = { type: 'footnoteReference', attrs: { id: '1' } }; + const positions: PositionMap = new WeakMap(); + + return { + node, + positions, + defaultFont: 'Calibri', + defaultSize: DEFAULT_FONT_SIZE, + inheritedMarks: [], + sdtMetadata: undefined, + hyperlinkConfig: { enableRichHyperlinks: false }, + themeColors: undefined, + runProperties: undefined, + paragraphProperties: undefined, + converterContext: {} as InlineConverterParams['converterContext'], + enableComments: false, + visitNode: vi.fn(), + bookmarks: undefined, + tabOrdinal: 0, + paragraphAttrs: {}, + nextBlockId: vi.fn(), + ...overrides, + } as InlineConverterParams; +} + +describe('buildReferenceMarkerRun', () => { + beforeEach(() => { + vi.mocked(textNodeToRun).mockReset(); + vi.mocked(textNodeToRun).mockImplementation( + (params: InlineConverterParams): TextRun => ({ + text: params.node.text || '', + fontFamily: params.defaultFont, + fontSize: params.defaultSize, + }), + ); + }); + + it('emits plain digit text, not Unicode superscript glyphs', () => { + const run = buildReferenceMarkerRun('1', makeParams()); + + expect(run.text).toBe('1'); + expect(run.text).not.toBe('¹'); + }); + + it('normalizes the default path to exactly one superscript treatment', () => { + const run = buildReferenceMarkerRun('1', makeParams()); + + expect(run.vertAlign).toBe('superscript'); + expect(run.baselineShift).toBeUndefined(); + expect(run.fontSize).toBe(DEFAULT_FONT_SIZE * SUBSCRIPT_SUPERSCRIPT_SCALE); + }); + + it('scales from the effective surrounding run size, not the paragraph default', () => { + vi.mocked(textNodeToRun) + .mockReturnValueOnce({ + text: '1', + fontFamily: 'Calibri', + fontSize: 16, + vertAlign: 'superscript', + }) + .mockReturnValueOnce({ + text: '1', + fontFamily: 'Calibri', + fontSize: 24, + }); + + const run = buildReferenceMarkerRun('1', makeParams({ defaultSize: 16 })); + + expect(run.fontSize).toBe(24 * SUBSCRIPT_SUPERSCRIPT_SCALE); + }); + + it('preserves explicit baseline shifts instead of forcing the default superscript path', () => { + vi.mocked(textNodeToRun).mockReturnValueOnce({ + text: '1', + fontFamily: 'Calibri', + fontSize: 18, + baselineShift: 3, + vertAlign: 'superscript', + }); + + const run = buildReferenceMarkerRun('1', makeParams()); + + expect(run.fontSize).toBe(18); + expect(run.baselineShift).toBe(3); + expect(vi.mocked(textNodeToRun)).toHaveBeenCalledTimes(1); + }); + + it('preserves inherited styling from the original run context', () => { + vi.mocked(textNodeToRun) + .mockReturnValueOnce({ + text: '1', + fontFamily: 'Arial', + fontSize: 20, + color: '#FF0000', + }) + .mockReturnValueOnce({ + text: '1', + fontFamily: 'Arial', + fontSize: 20, + }); + + const run = buildReferenceMarkerRun('1', makeParams()); + + expect(run.fontFamily).toBe('Arial'); + expect(run.color).toBe('#FF0000'); + }); + + it('copies PM positions from the reference node, not the synthetic text node', () => { + const node: PMNode = { type: 'footnoteReference', attrs: { id: '1' } }; + const positions: PositionMap = new WeakMap(); + positions.set(node, { start: 42, end: 43 }); + + const run = buildReferenceMarkerRun('1', makeParams({ node, positions })); + + expect(run.pmStart).toBe(42); + expect(run.pmEnd).toBe(43); + }); + + it('removes only vertical-positioning state from the normalization pass', () => { + const nodeMarks: PMMark[] = [ + { + type: 'textStyle', + attrs: { + fontFamily: 'Arial', + fontSize: '22pt', + vertAlign: 'superscript', + position: '3pt', + }, + }, + ]; + const inheritedMarks: PMMark[] = [ + { + type: 'textStyle', + attrs: { + color: '#FF0000', + vertAlign: 'superscript', + position: '2pt', + }, + }, + ]; + + buildReferenceMarkerRun( + '1', + makeParams({ + node: { type: 'footnoteReference', attrs: { id: '1' }, marks: nodeMarks }, + inheritedMarks, + runProperties: { + fontSize: 44, + fontFamily: { ascii: 'Arial' }, + vertAlign: 'superscript', + position: 6, + }, + }), + ); + + const normalizationPass = vi.mocked(textNodeToRun).mock.calls[1]?.[0]; + expect(normalizationPass).toBeDefined(); + expect(normalizationPass?.node).toEqual({ + type: 'text', + text: '1', + marks: [ + { + type: 'textStyle', + attrs: { + fontFamily: 'Arial', + fontSize: '22pt', + }, + }, + ], + }); + expect(normalizationPass?.inheritedMarks).toEqual([ + { + type: 'textStyle', + attrs: { + color: '#FF0000', + }, + }, + ]); + expect(normalizationPass?.runProperties).toEqual({ + fontSize: 44, + fontFamily: { ascii: 'Arial' }, + }); + }); +}); diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.ts new file mode 100644 index 0000000000..f9ac87387c --- /dev/null +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.ts @@ -0,0 +1,140 @@ +/** + * Shared helper for footnote and endnote reference markers. + * + * Reference markers must render as plain digits ("1", "2") with superscript + * positioning, NOT as Unicode superscript glyphs ("¹", "²"). This module + * applies exactly one superscript treatment for the default case, while still + * preserving explicit custom positioning when the source document provides it. + */ + +import type { TextRun } from '@superdoc/contracts'; +import type { RunProperties } from '@superdoc/style-engine/ooxml'; +import type { PMMark, PMNode } from '../../types.js'; +import { SUBSCRIPT_SUPERSCRIPT_SCALE } from '../../constants.js'; +import { textNodeToRun } from './text-run.js'; +import type { InlineConverterParams } from './common.js'; + +const buildSyntheticTextNode = (displayText: string, marks: PMMark[] | undefined): PMNode => ({ + type: 'text', + text: displayText, + marks: [...(marks ?? [])], +}); + +const isTextStyleMark = (mark: PMMark): boolean => mark.type === 'textStyle'; + +const stripVerticalPositioningFromMark = (mark: PMMark): PMMark | null => { + if (!isTextStyleMark(mark) || !mark.attrs) { + return mark; + } + + const sanitizedAttrs = { ...mark.attrs }; + delete sanitizedAttrs.vertAlign; + delete sanitizedAttrs.position; + + if (Object.keys(sanitizedAttrs).length === 0) { + return null; + } + + return { + ...mark, + attrs: sanitizedAttrs, + }; +}; + +const stripVerticalPositioningFromMarks = (marks: PMMark[] | undefined): PMMark[] => + (marks ?? []).flatMap((mark) => { + const sanitizedMark = stripVerticalPositioningFromMark(mark); + return sanitizedMark ? [sanitizedMark] : []; + }); + +const stripVerticalPositioningFromRunProperties = ( + runProperties: RunProperties | undefined, +): RunProperties | undefined => { + if (!runProperties) { + return undefined; + } + + const sanitizedRunProperties = { ...runProperties }; + delete sanitizedRunProperties.vertAlign; + delete sanitizedRunProperties.position; + return sanitizedRunProperties; +}; + +const copyReferencePmPositions = (run: TextRun, params: InlineConverterParams): TextRun => { + const refPos = params.positions.get(params.node); + if (!refPos) { + return run; + } + + return { + ...run, + pmStart: refPos.start, + pmEnd: refPos.end, + }; +}; + +const hasExplicitBaselineShift = (run: TextRun): boolean => + typeof run.baselineShift === 'number' && Number.isFinite(run.baselineShift); + +const resolveReferenceBaseFontSize = ( + runWithoutVerticalPositioning: TextRun, + originalRun: TextRun, + fallbackFontSize: number, +): number => { + if ( + typeof runWithoutVerticalPositioning.fontSize === 'number' && + Number.isFinite(runWithoutVerticalPositioning.fontSize) + ) { + return runWithoutVerticalPositioning.fontSize; + } + + if (typeof originalRun.fontSize === 'number' && Number.isFinite(originalRun.fontSize)) { + return originalRun.fontSize; + } + + return fallbackFontSize; +}; + +const buildOriginalReferenceRun = (displayText: string, params: InlineConverterParams): TextRun => + textNodeToRun({ + ...params, + node: buildSyntheticTextNode(displayText, params.node.marks), + }); + +const buildReferenceRunWithoutVerticalPositioning = (displayText: string, params: InlineConverterParams): TextRun => + textNodeToRun({ + ...params, + inheritedMarks: stripVerticalPositioningFromMarks(params.inheritedMarks), + runProperties: stripVerticalPositioningFromRunProperties(params.runProperties), + node: buildSyntheticTextNode(displayText, stripVerticalPositioningFromMarks(params.node.marks)), + }); + +/** + * Builds a TextRun for a footnote or endnote reference marker. + * + * Inherits font family, color, and other styling from the parent run context, + * then normalizes the marker rendering: + * - explicit custom baseline shifts are preserved as-is + * - the default path uses exactly one superscript treatment, sized from the + * effective surrounding run, not the paragraph default + */ +export function buildReferenceMarkerRun(displayText: string, params: InlineConverterParams): TextRun { + const originalRun = buildOriginalReferenceRun(displayText, params); + + if (hasExplicitBaselineShift(originalRun)) { + return copyReferencePmPositions(originalRun, params); + } + + const runWithoutVerticalPositioning = buildReferenceRunWithoutVerticalPositioning(displayText, params); + const baseFontSize = resolveReferenceBaseFontSize(runWithoutVerticalPositioning, originalRun, params.defaultSize); + + return copyReferencePmPositions( + { + ...originalRun, + vertAlign: 'superscript', + baselineShift: undefined, + fontSize: baseFontSize * SUBSCRIPT_SUPERSCRIPT_SCALE, + }, + params, + ); +} From 941f6b7eea18b6702b0984d6a941c645b56aaf2e Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Sat, 28 Mar 2026 12:38:54 -0700 Subject: [PATCH 2/3] fix: render footnote markers as scaled superscript digits --- .../measuring/dom/src/index.test.ts | 75 ++++++++++ .../layout-engine/measuring/dom/src/index.ts | 60 +++++--- .../painters/dom/src/renderer.ts | 50 +++++-- .../dom/src/text-style-rendering.test.ts | 9 +- .../layout/FootnotesBuilder.ts | 138 +++++++++++------- .../tests/FootnotesBuilder.test.ts | 39 ++++- 6 files changed, 282 insertions(+), 89 deletions(-) diff --git a/packages/layout-engine/measuring/dom/src/index.test.ts b/packages/layout-engine/measuring/dom/src/index.test.ts index f5fe5fc370..db7f401ce0 100644 --- a/packages/layout-engine/measuring/dom/src/index.test.ts +++ b/packages/layout-engine/measuring/dom/src/index.test.ts @@ -131,6 +131,81 @@ describe('measureBlock', () => { expect(measure.lines[0].width).toBeGreaterThan(0); }); + it('measures default superscript lines from the original base font size', async () => { + const baseBlock: FlowBlock = { + kind: 'paragraph', + id: 'base-superscript-line-height', + runs: [ + { + text: '1', + fontFamily: 'Arial', + fontSize: 16, + }, + ], + attrs: {}, + }; + + const superscriptBlock: FlowBlock = { + kind: 'paragraph', + id: 'superscript-line-height', + runs: [ + { + text: '1', + fontFamily: 'Arial', + fontSize: 16 * 0.65, + vertAlign: 'superscript', + }, + ], + attrs: {}, + }; + + const baseMeasure = expectParagraphMeasure(await measureBlock(baseBlock, 1000)); + const superscriptMeasure = expectParagraphMeasure(await measureBlock(superscriptBlock, 1000)); + + expect(superscriptMeasure.lines).toHaveLength(1); + expect(superscriptMeasure.lines[0].ascent).toBeCloseTo(baseMeasure.lines[0].ascent, 3); + expect(superscriptMeasure.lines[0].descent).toBeCloseTo(baseMeasure.lines[0].descent, 3); + expect(superscriptMeasure.lines[0].lineHeight).toBeCloseTo(baseMeasure.lines[0].lineHeight, 3); + }); + + it('does not unscale custom baselineShift runs during line measurement', async () => { + const baseBlock: FlowBlock = { + kind: 'paragraph', + id: 'base-baseline-shift-line-height', + runs: [ + { + text: 'shifted', + fontFamily: 'Arial', + fontSize: 16, + }, + ], + attrs: {}, + }; + + const shiftedBlock: FlowBlock = { + kind: 'paragraph', + id: 'baseline-shift-line-height', + runs: [ + { + text: 'shifted', + fontFamily: 'Arial', + fontSize: 16, + vertAlign: 'superscript', + baselineShift: 3, + }, + ], + attrs: {}, + }; + + const baseMeasure = expectParagraphMeasure(await measureBlock(baseBlock, 1000)); + const shiftedMeasure = expectParagraphMeasure(await measureBlock(shiftedBlock, 1000)); + + expect(shiftedMeasure.lines).toHaveLength(1); + expect(shiftedMeasure.lines[0].ascent).toBeCloseTo(baseMeasure.lines[0].ascent, 3); + expect(shiftedMeasure.lines[0].descent).toBeCloseTo(baseMeasure.lines[0].descent, 3); + expect(shiftedMeasure.lines[0].lineHeight).toBeCloseTo(baseMeasure.lines[0].lineHeight, 3); + }); + it('uses content width for wordLayout list first lines with standard hanging indent', async () => { // Standard hanging indent pattern: marker is positioned in the hanging area (left of text), // NOT inline with text. The marker doesn't consume horizontal space on the first line. diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index 7735154a33..124b7c413f 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -517,20 +517,46 @@ function calculateEmptyParagraphMetrics( }; } +/** + * Font size scaling factor applied by pm-adapter for superscript/subscript runs. + * Must match SUBSCRIPT_SUPERSCRIPT_SCALE in pm-adapter/src/constants.ts. + */ +const SUPERSCRIPT_SCALE = 0.65; + +const hasDefaultSuperscriptOrSubscript = (run: TextRun): boolean => + run.baselineShift == null && (run.vertAlign === 'superscript' || run.vertAlign === 'subscript'); + +/** + * Returns the font size to use for line height calculation. + * + * Only default superscript/subscript runs should be un-scaled here. When the + * source document provides an explicit baseline shift, pm-adapter preserves the + * original font size and stores the custom offset in `baselineShift`, so line + * measurement must use the run's actual font size as-is. + */ +function lineHeightFontSize(run: TextRun): number { + if (hasDefaultSuperscriptOrSubscript(run)) { + return run.fontSize / SUPERSCRIPT_SCALE; + } + return run.fontSize; +} + /** * Extract FontInfo from a TextRun for typography metrics calculation. + * Uses the line-height font size so that superscript/subscript runs + * produce metrics based on their original (un-scaled) base font. */ function getFontInfoFromRun(run: TextRun): FontInfo { return { fontFamily: normalizeFontFamily(run.fontFamily), - fontSize: normalizeFontSize(run.fontSize), + fontSize: normalizeFontSize(lineHeightFontSize(run)), bold: run.bold, italic: run.italic, }; } /** - * Update maxFontInfo when a new run has a larger font size. + * Update maxFontInfo when a new run has a larger effective font size for line height. * Returns the updated FontInfo if this run has the max font size, otherwise returns the existing info. */ function updateMaxFontInfo( @@ -538,7 +564,7 @@ function updateMaxFontInfo( currentMaxInfo: FontInfo | undefined, newRun: TextRun, ): FontInfo | undefined { - if (newRun.fontSize >= currentMaxSize) { + if (lineHeightFontSize(newRun) >= currentMaxSize) { return getFontInfoFromRun(newRun); } return currentMaxInfo; @@ -1821,7 +1847,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P toRun: runIndex, toChar: spacesEndChar, width: spacesWidth, - maxFontSize: run.fontSize, + maxFontSize: lineHeightFontSize(run), maxFontInfo: getFontInfoFromRun(run), maxWidth: getEffectiveWidth(lines.length === 0 ? initialAvailableWidth : bodyContentWidth), segments: [{ runIndex, fromChar: spacesStartChar, toChar: spacesEndChar, width: spacesWidth }], @@ -1854,7 +1880,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P toRun: runIndex, toChar: spacesEndChar, width: spacesWidth, - maxFontSize: run.fontSize, + maxFontSize: lineHeightFontSize(run), maxFontInfo: getFontInfoFromRun(run), maxWidth: getEffectiveWidth(bodyContentWidth), segments: [{ runIndex, fromChar: spacesStartChar, toChar: spacesEndChar, width: spacesWidth }], @@ -1865,7 +1891,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P currentLine.toChar = spacesEndChar; currentLine.width = roundValue(currentLine.width + boundarySpacing + spacesWidth); currentLine.maxFontInfo = updateMaxFontInfo(currentLine.maxFontSize, currentLine.maxFontInfo, run); - currentLine.maxFontSize = Math.max(currentLine.maxFontSize, run.fontSize); + currentLine.maxFontSize = Math.max(currentLine.maxFontSize, lineHeightFontSize(run)); appendSegment(currentLine.segments, runIndex, spacesStartChar, spacesEndChar, spacesWidth); currentLine.spaceCount += spacesLength; } @@ -1933,7 +1959,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P toRun: runIndex, toChar: spaceEndChar, width: singleSpaceWidth, - maxFontSize: run.fontSize, + maxFontSize: lineHeightFontSize(run), maxFontInfo: getFontInfoFromRun(run), maxWidth: getEffectiveWidth(lines.length === 0 ? initialAvailableWidth : bodyContentWidth), segments: [{ runIndex, fromChar: spaceStartChar, toChar: spaceEndChar, width: singleSpaceWidth }], @@ -1970,7 +1996,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P toRun: runIndex, toChar: spaceEndChar, width: singleSpaceWidth, - maxFontSize: run.fontSize, + maxFontSize: lineHeightFontSize(run), maxFontInfo: getFontInfoFromRun(run), maxWidth: getEffectiveWidth(bodyContentWidth), segments: [{ runIndex, fromChar: spaceStartChar, toChar: spaceEndChar, width: singleSpaceWidth }], @@ -1982,7 +2008,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P currentLine.toChar = spaceEndChar; currentLine.width = roundValue(currentLine.width + boundarySpacing + singleSpaceWidth); currentLine.maxFontInfo = updateMaxFontInfo(currentLine.maxFontSize, currentLine.maxFontInfo, run); - currentLine.maxFontSize = Math.max(currentLine.maxFontSize, run.fontSize); + currentLine.maxFontSize = Math.max(currentLine.maxFontSize, lineHeightFontSize(run)); // If in an active tab alignment group, use explicit X positioning let spaceExplicitX: number | undefined; if (inActiveTabGroup && activeTabGroup) { @@ -2075,7 +2101,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P currentLine.toRun = runIndex; currentLine.toChar = chunkEndChar; currentLine.width = roundValue(currentLine.width + chunk.width); - currentLine.maxFontSize = Math.max(currentLine.maxFontSize, run.fontSize); + currentLine.maxFontSize = Math.max(currentLine.maxFontSize, lineHeightFontSize(run)); currentLine.maxFontInfo = getFontInfoFromRun(run); currentLine.segments.push({ runIndex, @@ -2122,7 +2148,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P toRun: runIndex, toChar: chunkEndChar, width: chunk.width, - maxFontSize: run.fontSize, + maxFontSize: lineHeightFontSize(run), maxFontInfo: getFontInfoFromRun(run), maxWidth: getEffectiveWidth(contentWidth), segments: [{ runIndex, fromChar: chunkStartChar, toChar: chunkEndChar, width: chunk.width }], @@ -2170,7 +2196,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P toRun: runIndex, toChar: wordEndNoSpace, width: wordOnlyWidth, - maxFontSize: run.fontSize, + maxFontSize: lineHeightFontSize(run), maxFontInfo: getFontInfoFromRun(run), maxWidth: getEffectiveWidth(lines.length === 0 ? initialAvailableWidth : bodyContentWidth), segments: [{ runIndex, fromChar: wordStartChar, toChar: wordEndNoSpace, width: wordOnlyWidth }], @@ -2267,7 +2293,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P toRun: runIndex, toChar: wordEndNoSpace, width: wordOnlyWidth, - maxFontSize: run.fontSize, + maxFontSize: lineHeightFontSize(run), maxFontInfo: getFontInfoFromRun(run), maxWidth: getEffectiveWidth(bodyContentWidth), segments: [{ runIndex, fromChar: wordStartChar, toChar: wordEndNoSpace, width: wordOnlyWidth }], @@ -2300,7 +2326,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P currentLine.toChar = wordEndNoSpace; currentLine.width = roundValue(currentLine.width + boundarySpacing + wordOnlyWidth); currentLine.maxFontInfo = updateMaxFontInfo(currentLine.maxFontSize, currentLine.maxFontInfo, run); - currentLine.maxFontSize = Math.max(currentLine.maxFontSize, run.fontSize); + currentLine.maxFontSize = Math.max(currentLine.maxFontSize, lineHeightFontSize(run)); // Determine explicit X position: // - If in active tab group, use currentX from the group (for ALL words in group) // - Otherwise, only use segmentStartX for first word after a tab @@ -2352,7 +2378,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P } currentLine.width = roundValue(targetWidth); currentLine.maxFontInfo = updateMaxFontInfo(currentLine.maxFontSize, currentLine.maxFontInfo, run); - currentLine.maxFontSize = Math.max(currentLine.maxFontSize, run.fontSize); + currentLine.maxFontSize = Math.max(currentLine.maxFontSize, lineHeightFontSize(run)); appendSegment(currentLine.segments, runIndex, wordStartChar, newToChar, wordCommitWidth, explicitX); if (shouldIncludeDelimiterSpace) { currentLine.spaceCount += 1; @@ -2392,7 +2418,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P toRun: runIndex, toChar: charPosInRun, width: 0, - maxFontSize: run.fontSize, + maxFontSize: lineHeightFontSize(run), maxFontInfo: getFontInfoFromRun(run), maxWidth: getEffectiveWidth(lines.length === 0 ? initialAvailableWidth : bodyContentWidth), segments: [], @@ -2411,7 +2437,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P currentLine.width = roundValue(currentLine.width + tabAdvance); currentLine.maxFontInfo = updateMaxFontInfo(currentLine.maxFontSize, currentLine.maxFontInfo, run); - currentLine.maxFontSize = Math.max(currentLine.maxFontSize, run.fontSize); + currentLine.maxFontSize = Math.max(currentLine.maxFontSize, lineHeightFontSize(run)); currentLine.toRun = runIndex; currentLine.toChar = charPosInRun; charPosInRun += 1; diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index c9162c54f6..ae6dfd5b98 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -7261,6 +7261,45 @@ const deriveBlockVersion = (block: FlowBlock): string => { return block.id; }; +const SUPERSCRIPT_SCALE = 0.65; +const DEFAULT_SUPERSCRIPT_RAISE_RATIO = 0.33; +const DEFAULT_SUBSCRIPT_LOWER_RATIO = 0.14; + +const hasVerticalPositioning = (run: TextRun): boolean => + (run.baselineShift != null && Number.isFinite(run.baselineShift)) || + run.vertAlign === 'superscript' || + run.vertAlign === 'subscript'; + +const applyRunVerticalPositioning = (element: HTMLElement, run: TextRun): void => { + // Vertically shifted runs should use a tight inline box. If they inherit the + // parent line's full line-height, the glyph remains visually low inside an + // oversized inline box even when the superscript/subscript offset is correct. + if (hasVerticalPositioning(run)) { + element.style.lineHeight = '1'; + } + + if (run.baselineShift != null && Number.isFinite(run.baselineShift)) { + element.style.verticalAlign = `${run.baselineShift}pt`; + return; + } + + if (run.vertAlign === 'superscript') { + const baseFontSize = run.fontSize / SUPERSCRIPT_SCALE; + element.style.verticalAlign = `${baseFontSize * DEFAULT_SUPERSCRIPT_RAISE_RATIO}px`; + return; + } + + if (run.vertAlign === 'subscript') { + const baseFontSize = run.fontSize / SUPERSCRIPT_SCALE; + element.style.verticalAlign = `${-(baseFontSize * DEFAULT_SUBSCRIPT_LOWER_RATIO)}px`; + return; + } + + if (run.vertAlign === 'baseline') { + element.style.verticalAlign = 'baseline'; + } +}; + /** * Applies run styling properties to a DOM element. * @@ -7320,16 +7359,7 @@ const applyRunStyles = (element: HTMLElement, run: Run, _isLink = false): void = element.style.textDecorationLine = decorations.join(' '); } - // Vertical alignment: custom baseline offset takes precedence over vertAlign - if (run.baselineShift != null && Number.isFinite(run.baselineShift)) { - element.style.verticalAlign = `${run.baselineShift}pt`; - } else if (run.vertAlign === 'superscript') { - element.style.verticalAlign = 'super'; - } else if (run.vertAlign === 'subscript') { - element.style.verticalAlign = 'sub'; - } else if (run.vertAlign === 'baseline') { - element.style.verticalAlign = 'baseline'; - } + applyRunVerticalPositioning(element, run); }; interface CommentHighlightResult { diff --git a/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts b/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts index 67bf51ab9e..5b758d1510 100644 --- a/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts +++ b/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts @@ -346,7 +346,9 @@ describe('DomPainter text style CSS rendering', () => { const span = container.querySelector('span'); expect(span).toBeTruthy(); - expect(span?.style.verticalAlign).toBe('super'); + // Computed pixel offset: baseFontSize (10.4 / 0.65 = 16) * 0.33 = 5.28 + expect(span?.style.lineHeight).toBe('1'); + expect(span?.style.verticalAlign).toBe('5.28px'); }); it('should apply vertical-align sub for subscript', () => { @@ -373,7 +375,9 @@ describe('DomPainter text style CSS rendering', () => { const span = container.querySelector('span'); expect(span).toBeTruthy(); - expect(span?.style.verticalAlign).toBe('sub'); + // Computed pixel offset: -(baseFontSize (10.4 / 0.65 = 16) * 0.14) = -2.24 + expect(span?.style.lineHeight).toBe('1'); + expect(span?.style.verticalAlign).toBe('-2.24px'); }); it('should apply vertical-align with pt offset for baselineShift', () => { @@ -400,6 +404,7 @@ describe('DomPainter text style CSS rendering', () => { const span = container.querySelector('span'); expect(span).toBeTruthy(); + expect(span?.style.lineHeight).toBe('1'); expect(span?.style.verticalAlign).toBe('3pt'); }); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/layout/FootnotesBuilder.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/layout/FootnotesBuilder.ts index 35790466a0..9a73cadd3b 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/layout/FootnotesBuilder.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/layout/FootnotesBuilder.ts @@ -11,7 +11,7 @@ * placement, and click-to-position functionality. * * - `data-sd-footnote-number`: A data attribute marking the superscript number - * run (e.g., "¹") at the start of footnote content. Used to distinguish the + * run (e.g., "1") at the start of footnote content. Used to distinguish the * marker from actual footnote text during rendering and selection. * * @module presentation-editor/layout/FootnotesBuilder @@ -20,6 +20,7 @@ import type { EditorState } from 'prosemirror-state'; import type { FlowBlock } from '@superdoc/contracts'; import { toFlowBlocks, type ConverterContext } from '@superdoc/pm-adapter'; +import { SUBSCRIPT_SUPERSCRIPT_SCALE } from '@superdoc/pm-adapter/constants.js'; import type { FootnoteReference, FootnotesLayoutInput } from '../types.js'; import { findNoteEntryById } from '../../../document-api-adapters/helpers/note-entry-lookup.js'; @@ -42,7 +43,12 @@ type Run = { text?: string; fontFamily?: string; fontSize?: number; + bold?: boolean; + italic?: boolean; + letterSpacing?: number; color?: unknown; + vertAlign?: 'superscript' | 'subscript' | 'baseline'; + baselineShift?: number; pmStart?: number | null; pmEnd?: number | null; dataAttrs?: Record; @@ -54,6 +60,10 @@ type ParagraphBlock = FlowBlock & { runs?: Run[]; }; +const FOOTNOTE_MARKER_DATA_ATTR = 'data-sd-footnote-number'; +const DEFAULT_MARKER_FONT_FAMILY = 'Arial'; +const DEFAULT_MARKER_FONT_SIZE = 12; + // ============================================================================= // Public API // ============================================================================= @@ -154,7 +164,7 @@ export function buildFootnotesInput( * @returns True if the run has the footnote marker data attribute */ function isFootnoteMarker(run: Run): boolean { - return Boolean(run.dataAttrs?.['data-sd-footnote-number']); + return Boolean(run.dataAttrs?.[FOOTNOTE_MARKER_DATA_ATTR]); } /** @@ -192,34 +202,13 @@ function resolveDisplayNumber(id: string, footnoteNumberById: Record = { - '0': '⁰', - '1': '¹', - '2': '²', - '3': '³', - '4': '⁴', - '5': '⁵', - '6': '⁶', - '7': '⁷', - '8': '⁸', - '9': '⁹', - }; - const str = String(value ?? ''); - return str - .split('') - .map((ch) => SUPERSCRIPT_MAP[ch] ?? ch) - .join(''); +function resolveMarkerText(value: unknown): string { + return String(value ?? ''); } /** @@ -249,12 +238,70 @@ function computeMarkerPositions( return { pmStart, pmEnd }; } +function resolveMarkerFontFamily(firstTextRun: Run | undefined): string { + return typeof firstTextRun?.fontFamily === 'string' ? firstTextRun.fontFamily : DEFAULT_MARKER_FONT_FAMILY; +} + +function resolveMarkerBaseFontSize(firstTextRun: Run | undefined): number { + if ( + typeof firstTextRun?.fontSize === 'number' && + Number.isFinite(firstTextRun.fontSize) && + firstTextRun.fontSize > 0 + ) { + return firstTextRun.fontSize; + } + + return DEFAULT_MARKER_FONT_SIZE; +} + +function buildMarkerRun( + markerText: string, + firstTextRun: Run | undefined, + positions: { pmStart: number | null; pmEnd: number | null }, +): Run { + const markerRun: Run = { + kind: 'text', + text: markerText, + dataAttrs: { [FOOTNOTE_MARKER_DATA_ATTR]: 'true' }, + fontFamily: resolveMarkerFontFamily(firstTextRun), + fontSize: resolveMarkerBaseFontSize(firstTextRun) * SUBSCRIPT_SUPERSCRIPT_SCALE, + vertAlign: 'superscript', + }; + + if (typeof firstTextRun?.bold === 'boolean') markerRun.bold = firstTextRun.bold; + if (typeof firstTextRun?.italic === 'boolean') markerRun.italic = firstTextRun.italic; + if (typeof firstTextRun?.letterSpacing === 'number' && Number.isFinite(firstTextRun.letterSpacing)) { + markerRun.letterSpacing = firstTextRun.letterSpacing; + } + if (firstTextRun?.color != null) markerRun.color = firstTextRun.color; + if (positions.pmStart != null) markerRun.pmStart = positions.pmStart; + if (positions.pmEnd != null) markerRun.pmEnd = positions.pmEnd; + + return markerRun; +} + +function syncMarkerRun(target: Run, source: Run): void { + target.kind = source.kind; + target.text = source.text; + target.dataAttrs = source.dataAttrs; + target.fontFamily = source.fontFamily; + target.fontSize = source.fontSize; + target.bold = source.bold; + target.italic = source.italic; + target.letterSpacing = source.letterSpacing; + target.color = source.color; + target.vertAlign = source.vertAlign; + target.baselineShift = source.baselineShift; + target.pmStart = source.pmStart ?? target.pmStart; + target.pmEnd = source.pmEnd ?? target.pmEnd; +} + /** * Ensures a footnote block has a superscript marker at the start. * * Word and other editors display footnote content with a leading superscript - * number (e.g., "¹ This is the footnote text."). This function prepends that - * marker to the first paragraph's runs. + * number rendered as a normal digit with superscript styling. This function + * prepends that marker to the first paragraph's runs. * * If a marker already exists, updates its PM positions if missing. * Modifies the blocks array in place. @@ -273,41 +320,22 @@ function ensureFootnoteMarker( const runs: Run[] = Array.isArray(firstParagraph.runs) ? firstParagraph.runs : []; const displayNumber = resolveDisplayNumber(id, footnoteNumberById); - const markerText = toSuperscriptDigits(displayNumber); + const markerText = resolveMarkerText(displayNumber); const baseRun = findRunWithPositions(runs); - const { pmStart, pmEnd } = computeMarkerPositions(baseRun, markerText.length); + const positions = computeMarkerPositions(baseRun, markerText.length); + const firstTextRun = runs.find((run) => typeof run.text === 'string' && !isFootnoteMarker(run)); + const normalizedMarkerRun = buildMarkerRun(markerText, firstTextRun, positions); // Check if marker already exists const existingMarker = runs.find(isFootnoteMarker); if (existingMarker) { - // Update position info on existing marker if missing - if (pmStart != null && pmEnd != null) { - if (existingMarker.pmStart == null) existingMarker.pmStart = pmStart; - if (existingMarker.pmEnd == null) existingMarker.pmEnd = pmEnd; - } + syncMarkerRun(existingMarker, normalizedMarkerRun); return; } - // Find first text run to inherit font styling from - const firstTextRun = runs.find((r) => typeof r.text === 'string'); - - // Build the marker run - const markerRun: Run = { - kind: 'text', - text: markerText, - dataAttrs: { 'data-sd-footnote-number': 'true' }, - fontFamily: typeof firstTextRun?.fontFamily === 'string' ? firstTextRun.fontFamily : 'Arial', - fontSize: - typeof firstTextRun?.fontSize === 'number' && Number.isFinite(firstTextRun.fontSize) ? firstTextRun.fontSize : 12, - }; - - if (pmStart != null) markerRun.pmStart = pmStart; - if (pmEnd != null) markerRun.pmEnd = pmEnd; - if (firstTextRun?.color != null) markerRun.color = firstTextRun.color; - // Insert marker at the very start of runs - runs.unshift(markerRun); + runs.unshift(normalizedMarkerRun); // Cast needed: local Run type is structurally compatible but not identical // to the FlowBlock's Run type from @superdoc/contracts (firstParagraph as { runs: Run[] }).runs = runs; diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/FootnotesBuilder.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/FootnotesBuilder.test.ts index a70261b062..d4d0a7da6c 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/FootnotesBuilder.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/FootnotesBuilder.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi } from 'vitest'; import type { EditorState } from 'prosemirror-state'; import { buildFootnotesInput, type ConverterLike } from '../layout/FootnotesBuilder.js'; import type { ConverterContext } from '@superdoc/pm-adapter'; +import { SUBSCRIPT_SUPERSCRIPT_SCALE } from '@superdoc/pm-adapter/constants.js'; // Mock toFlowBlocks vi.mock('@superdoc/pm-adapter', async (importOriginal) => { @@ -53,6 +54,10 @@ function createMockConverterContext(footnoteNumberById: Record): return { footnoteNumberById } as ConverterContext; } +function blocksFromResult(result: ReturnType) { + return result?.blocksById.get('1'); +} + // ============================================================================= // Tests // ============================================================================= @@ -175,10 +180,34 @@ describe('buildFootnotesInput', () => { const firstRun = (blocks?.[0] as { runs?: Array<{ text?: string; dataAttrs?: Record }> }) ?.runs?.[0]; - expect(firstRun?.text).toBe('¹'); + expect(firstRun?.text).toBe('1'); expect(firstRun?.dataAttrs?.['data-sd-footnote-number']).toBe('true'); }); + it('builds the marker as a scaled superscript run instead of a Unicode superscript glyph', () => { + const editorState = createMockEditorState([{ id: '1', pos: 10 }]); + const converter = createMockConverter([ + { id: '1', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Note' }] }] }, + ]); + const context = createMockConverterContext({ '1': 1 }); + + const result = buildFootnotesInput(editorState, converter, context, undefined); + + const firstRun = ( + blocksFromResult(result)?.[0] as { + runs?: Array<{ + text?: string; + fontSize?: number; + vertAlign?: string; + }>; + } + )?.runs?.[0]; + + expect(firstRun?.text).toBe('1'); + expect(firstRun?.fontSize).toBe(12 * SUBSCRIPT_SUPERSCRIPT_SCALE); + expect(firstRun?.vertAlign).toBe('superscript'); + }); + it('uses correct display number from context', () => { const editorState = createMockEditorState([{ id: '5', pos: 10 }]); const converter = createMockConverter([ @@ -190,7 +219,7 @@ describe('buildFootnotesInput', () => { const blocks = result?.blocksById.get('5'); const firstRun = (blocks?.[0] as { runs?: Array<{ text?: string }> })?.runs?.[0]; - expect(firstRun?.text).toBe('³'); + expect(firstRun?.text).toBe('3'); }); it('handles multi-digit display numbers', () => { @@ -204,7 +233,7 @@ describe('buildFootnotesInput', () => { const blocks = result?.blocksById.get('1'); const firstRun = (blocks?.[0] as { runs?: Array<{ text?: string }> })?.runs?.[0]; - expect(firstRun?.text).toBe('¹²³'); + expect(firstRun?.text).toBe('123'); }); it('defaults to 1 when footnoteNumberById is missing entry', () => { @@ -218,7 +247,7 @@ describe('buildFootnotesInput', () => { const blocks = result?.blocksById.get('99'); const firstRun = (blocks?.[0] as { runs?: Array<{ text?: string }> })?.runs?.[0]; - expect(firstRun?.text).toBe('¹'); + expect(firstRun?.text).toBe('1'); }); it('defaults to 1 when converterContext is undefined', () => { @@ -231,7 +260,7 @@ describe('buildFootnotesInput', () => { const blocks = result?.blocksById.get('1'); const firstRun = (blocks?.[0] as { runs?: Array<{ text?: string }> })?.runs?.[0]; - expect(firstRun?.text).toBe('¹'); + expect(firstRun?.text).toBe('1'); }); }); From 764fcd03b252d14505aef7ed2bbb84bbeeefb6f4 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Sat, 28 Mar 2026 13:40:01 -0700 Subject: [PATCH 3/3] fix: treat zero baseline shift as a no-op for superscript rendering --- devtools/visual-testing/pnpm-lock.yaml | 2 +- packages/layout-engine/contracts/src/index.ts | 15 +- .../contracts/src/vertical-text.test.ts | 179 ++++++++++++++++++ .../contracts/src/vertical-text.ts | 78 ++++++++ .../measuring/dom/src/index.test.ts | 37 ++++ .../layout-engine/measuring/dom/src/index.ts | 23 +-- .../painters/dom/src/renderer.ts | 16 +- .../dom/src/text-style-rendering.test.ts | 29 +++ .../src/attributes/paragraph.test.ts | 12 ++ .../pm-adapter/src/attributes/paragraph.ts | 32 ++-- .../layout-engine/pm-adapter/src/constants.ts | 7 +- .../reference-marker.test.ts | 23 +++ .../inline-converters/reference-marker.ts | 7 +- .../pm-adapter/src/marks/application.test.ts | 13 ++ .../pm-adapter/src/marks/application.ts | 27 ++- .../editors/v1/core/super-converter/styles.js | 27 +-- .../v1/core/super-converter/styles.test.js | 10 +- .../v1/extensions/text-style/text-style.js | 20 +- .../v1/extensions/types/mark-attributes.ts | 5 +- 19 files changed, 471 insertions(+), 91 deletions(-) create mode 100644 packages/layout-engine/contracts/src/vertical-text.test.ts create mode 100644 packages/layout-engine/contracts/src/vertical-text.ts diff --git a/devtools/visual-testing/pnpm-lock.yaml b/devtools/visual-testing/pnpm-lock.yaml index bd42bdc85f..f323ab8edb 100644 --- a/devtools/visual-testing/pnpm-lock.yaml +++ b/devtools/visual-testing/pnpm-lock.yaml @@ -2006,7 +2006,7 @@ packages: resolution: {integrity: sha512-l63NF9y/cLROq/yqKXSLtcMeeyOfnSQlfMSlzFt/K73oIaD8DGaQWd7Z34X9GPiKqP5rbSh84Hl4bOlLcjiSrQ==} superdoc@file:../../packages/superdoc/superdoc.tgz: - resolution: {integrity: sha512-VSxckZlguegAfx9nYqI4aDDbix9Kw6Qr9aB5GPYlaOBsx3EDtMDHlRJGnkXHNg1QAEUkQtayHMCx54uZSYcDvg==, tarball: file:../../packages/superdoc/superdoc.tgz} + resolution: {integrity: sha512-/p1mQ0yQx2FFvpmr6YJ//J/Tz/SoW4a40WWzOpjD7+yBXdcOQbT93KU3tkDRIPcTGk93DHdHXkeseWemSTwZNg==, tarball: file:../../packages/superdoc/superdoc.tgz} version: 1.23.0 peerDependencies: '@hocuspocus/provider': ^2.13.6 diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 2f80b5c8ec..1464a2d1a5 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -39,6 +39,16 @@ export { formatInsetClipPathTransform, type InsetClipPathScale, } from './clip-path-inset.js'; +export { + SUBSCRIPT_SUPERSCRIPT_SCALE, + normalizeBaselineShift, + hasExplicitBaselineShift, + isSuperscriptOrSubscript, + usesDefaultScriptLayout, + scaleFontSizeForVerticalText, + resolveBaseFontSizeForVerticalText, + type VerticalTextAlign, +} from './vertical-text.js'; export { computeFragmentPmRange, computeLinePmRange, type LinePmRange } from './pm-range.js'; export { cloneColumnLayout, normalizeColumnLayout, widthsEqual } from './column-layout.js'; @@ -200,7 +210,10 @@ export type RunMarks = { textTransform?: 'uppercase' | 'lowercase' | 'capitalize' | 'none'; /** Vertical alignment for superscript/subscript text. */ vertAlign?: 'superscript' | 'subscript' | 'baseline'; - /** Custom baseline shift in points (positive = raise, negative = lower). Takes precedence over vertAlign for positioning. */ + /** + * Explicit baseline shift in points (positive = raise, negative = lower). + * Rendering normalizes a shift of zero to "no explicit shift". + */ baselineShift?: number; }; diff --git a/packages/layout-engine/contracts/src/vertical-text.test.ts b/packages/layout-engine/contracts/src/vertical-text.test.ts new file mode 100644 index 0000000000..5d23a6001b --- /dev/null +++ b/packages/layout-engine/contracts/src/vertical-text.test.ts @@ -0,0 +1,179 @@ +import { describe, expect, it } from 'vitest'; +import { + SUBSCRIPT_SUPERSCRIPT_SCALE, + normalizeBaselineShift, + hasExplicitBaselineShift, + isSuperscriptOrSubscript, + usesDefaultScriptLayout, + scaleFontSizeForVerticalText, + resolveBaseFontSizeForVerticalText, +} from './vertical-text.js'; + +describe('normalizeBaselineShift', () => { + it('returns undefined for null', () => { + expect(normalizeBaselineShift(null)).toBeUndefined(); + }); + + it('returns undefined for undefined', () => { + expect(normalizeBaselineShift(undefined)).toBeUndefined(); + }); + + it('returns undefined for NaN', () => { + expect(normalizeBaselineShift(NaN)).toBeUndefined(); + }); + + it('returns undefined for Infinity', () => { + expect(normalizeBaselineShift(Infinity)).toBeUndefined(); + }); + + it('returns undefined for zero (identity value)', () => { + expect(normalizeBaselineShift(0)).toBeUndefined(); + }); + + it('returns undefined for near-zero values within epsilon', () => { + expect(normalizeBaselineShift(1e-7)).toBeUndefined(); + expect(normalizeBaselineShift(-1e-7)).toBeUndefined(); + }); + + it('returns the value for positive shifts', () => { + expect(normalizeBaselineShift(3)).toBe(3); + }); + + it('returns the value for negative shifts', () => { + expect(normalizeBaselineShift(-1.5)).toBe(-1.5); + }); + + it('returns the value for small but non-zero shifts', () => { + expect(normalizeBaselineShift(0.01)).toBe(0.01); + }); +}); + +describe('hasExplicitBaselineShift', () => { + it('returns false for null/undefined/zero', () => { + expect(hasExplicitBaselineShift(null)).toBe(false); + expect(hasExplicitBaselineShift(undefined)).toBe(false); + expect(hasExplicitBaselineShift(0)).toBe(false); + }); + + it('returns true for non-zero finite values', () => { + expect(hasExplicitBaselineShift(3)).toBe(true); + expect(hasExplicitBaselineShift(-1.5)).toBe(true); + }); +}); + +describe('isSuperscriptOrSubscript', () => { + it('returns true for superscript', () => { + expect(isSuperscriptOrSubscript('superscript')).toBe(true); + }); + + it('returns true for subscript', () => { + expect(isSuperscriptOrSubscript('subscript')).toBe(true); + }); + + it('returns false for baseline', () => { + expect(isSuperscriptOrSubscript('baseline')).toBe(false); + }); + + it('returns false for null/undefined', () => { + expect(isSuperscriptOrSubscript(null)).toBe(false); + expect(isSuperscriptOrSubscript(undefined)).toBe(false); + }); +}); + +describe('usesDefaultScriptLayout', () => { + it('returns true for superscript without explicit shift', () => { + expect(usesDefaultScriptLayout({ vertAlign: 'superscript' })).toBe(true); + }); + + it('returns true for subscript without explicit shift', () => { + expect(usesDefaultScriptLayout({ vertAlign: 'subscript' })).toBe(true); + }); + + it('returns false for superscript with explicit shift', () => { + expect(usesDefaultScriptLayout({ vertAlign: 'superscript', baselineShift: 3 })).toBe(false); + }); + + it('returns true for superscript with zero shift (identity)', () => { + expect(usesDefaultScriptLayout({ vertAlign: 'superscript', baselineShift: 0 })).toBe(true); + }); + + it('returns false for baseline', () => { + expect(usesDefaultScriptLayout({ vertAlign: 'baseline' })).toBe(false); + }); + + it('returns false when no vertAlign', () => { + expect(usesDefaultScriptLayout({})).toBe(false); + expect(usesDefaultScriptLayout({ baselineShift: 3 })).toBe(false); + }); +}); + +describe('scaleFontSizeForVerticalText', () => { + it('scales font size for default superscript', () => { + expect(scaleFontSizeForVerticalText(16, { vertAlign: 'superscript' })).toBeCloseTo( + 16 * SUBSCRIPT_SUPERSCRIPT_SCALE, + ); + }); + + it('scales font size for default subscript', () => { + expect(scaleFontSizeForVerticalText(16, { vertAlign: 'subscript' })).toBeCloseTo(16 * SUBSCRIPT_SUPERSCRIPT_SCALE); + }); + + it('does not scale when explicit shift is present', () => { + expect(scaleFontSizeForVerticalText(16, { vertAlign: 'superscript', baselineShift: 3 })).toBe(16); + }); + + it('scales when shift is zero (identity)', () => { + expect(scaleFontSizeForVerticalText(16, { vertAlign: 'superscript', baselineShift: 0 })).toBeCloseTo( + 16 * SUBSCRIPT_SUPERSCRIPT_SCALE, + ); + }); + + it('does not scale for baseline', () => { + expect(scaleFontSizeForVerticalText(16, { vertAlign: 'baseline' })).toBe(16); + }); + + it('does not scale when no vertAlign', () => { + expect(scaleFontSizeForVerticalText(16, {})).toBe(16); + }); + + it('passes through non-finite values unchanged', () => { + expect(scaleFontSizeForVerticalText(NaN, { vertAlign: 'superscript' })).toBeNaN(); + expect(scaleFontSizeForVerticalText(Infinity, { vertAlign: 'superscript' })).toBe(Infinity); + }); +}); + +describe('resolveBaseFontSizeForVerticalText', () => { + it('un-scales default superscript font size', () => { + const scaled = 16 * SUBSCRIPT_SUPERSCRIPT_SCALE; + expect(resolveBaseFontSizeForVerticalText(scaled, { vertAlign: 'superscript' })).toBeCloseTo(16); + }); + + it('un-scales default subscript font size', () => { + const scaled = 16 * SUBSCRIPT_SUPERSCRIPT_SCALE; + expect(resolveBaseFontSizeForVerticalText(scaled, { vertAlign: 'subscript' })).toBeCloseTo(16); + }); + + it('returns font size unchanged when explicit shift is present', () => { + expect(resolveBaseFontSizeForVerticalText(16, { vertAlign: 'superscript', baselineShift: 3 })).toBe(16); + }); + + it('un-scales when shift is zero (identity)', () => { + const scaled = 16 * SUBSCRIPT_SUPERSCRIPT_SCALE; + expect(resolveBaseFontSizeForVerticalText(scaled, { vertAlign: 'superscript', baselineShift: 0 })).toBeCloseTo(16); + }); + + it('returns font size unchanged for baseline', () => { + expect(resolveBaseFontSizeForVerticalText(16, { vertAlign: 'baseline' })).toBe(16); + }); + + it('passes through non-finite values unchanged', () => { + expect(resolveBaseFontSizeForVerticalText(NaN, { vertAlign: 'superscript' })).toBeNaN(); + }); + + it('roundtrips with scaleFontSizeForVerticalText', () => { + const formatting = { vertAlign: 'superscript' as const }; + const original = 24; + const scaled = scaleFontSizeForVerticalText(original, formatting); + expect(resolveBaseFontSizeForVerticalText(scaled, formatting)).toBeCloseTo(original); + }); +}); diff --git a/packages/layout-engine/contracts/src/vertical-text.ts b/packages/layout-engine/contracts/src/vertical-text.ts new file mode 100644 index 0000000000..f281077f80 --- /dev/null +++ b/packages/layout-engine/contracts/src/vertical-text.ts @@ -0,0 +1,78 @@ +/** + * Shared vertical-text helpers for superscript, subscript, and explicit baseline shifts. + * + * OOXML allows both semantic vertical alignment (`vertAlign`) and an explicit + * position offset (`position`). During rendering, a zero offset is an identity + * value and should behave the same as an absent offset. + */ + +export type VerticalTextAlign = 'superscript' | 'subscript' | 'baseline'; + +type VerticalTextFormatting = { + vertAlign?: VerticalTextAlign | null; + baselineShift?: number | null; +}; + +/** + * Font size scaling factor for default superscript/subscript rendering. + * Matches Microsoft Word's default visual behavior closely enough for layout. + */ +export const SUBSCRIPT_SUPERSCRIPT_SCALE = 0.65; + +const BASELINE_SHIFT_EPSILON = 1e-6; + +/** + * Normalizes explicit baseline shifts for rendering. + * + * A numeric shift of zero is a no-op and should not override semantic + * superscript/subscript styling. This preserves the raw OOXML value for + * round-tripping while giving the renderer a clean intent model. + */ +export function normalizeBaselineShift(baselineShift: number | null | undefined): number | undefined { + if (!Number.isFinite(baselineShift)) { + return undefined; + } + + const normalizedShift = baselineShift as number; + return Math.abs(normalizedShift) <= BASELINE_SHIFT_EPSILON ? undefined : normalizedShift; +} + +export function hasExplicitBaselineShift(baselineShift: number | null | undefined): boolean { + return normalizeBaselineShift(baselineShift) != null; +} + +export function isSuperscriptOrSubscript(vertAlign: VerticalTextAlign | null | undefined): boolean { + return vertAlign === 'superscript' || vertAlign === 'subscript'; +} + +/** + * Returns true when the run should use the default superscript/subscript + * presentation path: scaled font size plus the renderer's default raise/lower. + */ +export function usesDefaultScriptLayout(formatting: VerticalTextFormatting): boolean { + return isSuperscriptOrSubscript(formatting.vertAlign) && !hasExplicitBaselineShift(formatting.baselineShift); +} + +/** + * Applies default superscript/subscript font scaling when the run uses the + * default semantic layout path. + */ +export function scaleFontSizeForVerticalText(fontSize: number, formatting: VerticalTextFormatting): number { + if (!Number.isFinite(fontSize)) { + return fontSize; + } + + return usesDefaultScriptLayout(formatting) ? fontSize * SUBSCRIPT_SUPERSCRIPT_SCALE : fontSize; +} + +/** + * Returns the original base font size for runs that already carry scaled + * superscript/subscript text metrics. + */ +export function resolveBaseFontSizeForVerticalText(fontSize: number, formatting: VerticalTextFormatting): number { + if (!Number.isFinite(fontSize)) { + return fontSize; + } + + return usesDefaultScriptLayout(formatting) ? fontSize / SUBSCRIPT_SUPERSCRIPT_SCALE : fontSize; +} diff --git a/packages/layout-engine/measuring/dom/src/index.test.ts b/packages/layout-engine/measuring/dom/src/index.test.ts index db7f401ce0..c4d26ea236 100644 --- a/packages/layout-engine/measuring/dom/src/index.test.ts +++ b/packages/layout-engine/measuring/dom/src/index.test.ts @@ -206,6 +206,43 @@ describe('measureBlock', () => { expect(shiftedMeasure.lines[0].lineHeight).toBeCloseTo(baseMeasure.lines[0].lineHeight, 3); }); + it('treats zero baselineShift as identity during superscript measurement', async () => { + const baseBlock: FlowBlock = { + kind: 'paragraph', + id: 'base-zero-baseline-shift-line-height', + runs: [ + { + text: '1', + fontFamily: 'Arial', + fontSize: 16, + }, + ], + attrs: {}, + }; + + const superscriptWithZeroShiftBlock: FlowBlock = { + kind: 'paragraph', + id: 'zero-baseline-shift-line-height', + runs: [ + { + text: '1', + fontFamily: 'Arial', + fontSize: 16 * 0.65, + vertAlign: 'superscript', + baselineShift: 0, + }, + ], + attrs: {}, + }; + + const baseMeasure = expectParagraphMeasure(await measureBlock(baseBlock, 1000)); + const superscriptMeasure = expectParagraphMeasure(await measureBlock(superscriptWithZeroShiftBlock, 1000)); + + expect(superscriptMeasure.lines[0].ascent).toBeCloseTo(baseMeasure.lines[0].ascent, 3); + expect(superscriptMeasure.lines[0].descent).toBeCloseTo(baseMeasure.lines[0].descent, 3); + expect(superscriptMeasure.lines[0].lineHeight).toBeCloseTo(baseMeasure.lines[0].lineHeight, 3); + }); + it('uses content width for wordLayout list first lines with standard hanging indent', async () => { // Standard hanging indent pattern: marker is positioned in the hanging area (left of text), // NOT inline with text. The marker doesn't consume horizontal space on the first line. diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index 124b7c413f..82e6f8a5d6 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -65,6 +65,7 @@ import { type TableBorderValue, effectiveTableCellSpacing, LeaderDecoration, + resolveBaseFontSizeForVerticalText, } from '@superdoc/contracts'; import type { WordParagraphLayoutOutput } from '@superdoc/word-layout'; import { @@ -517,28 +518,8 @@ function calculateEmptyParagraphMetrics( }; } -/** - * Font size scaling factor applied by pm-adapter for superscript/subscript runs. - * Must match SUBSCRIPT_SUPERSCRIPT_SCALE in pm-adapter/src/constants.ts. - */ -const SUPERSCRIPT_SCALE = 0.65; - -const hasDefaultSuperscriptOrSubscript = (run: TextRun): boolean => - run.baselineShift == null && (run.vertAlign === 'superscript' || run.vertAlign === 'subscript'); - -/** - * Returns the font size to use for line height calculation. - * - * Only default superscript/subscript runs should be un-scaled here. When the - * source document provides an explicit baseline shift, pm-adapter preserves the - * original font size and stores the custom offset in `baselineShift`, so line - * measurement must use the run's actual font size as-is. - */ function lineHeightFontSize(run: TextRun): number { - if (hasDefaultSuperscriptOrSubscript(run)) { - return run.fontSize / SUPERSCRIPT_SCALE; - } - return run.fontSize; + return resolveBaseFontSizeForVerticalText(run.fontSize, run); } /** diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index ae6dfd5b98..81815dfe91 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -59,6 +59,8 @@ import { calculateJustifySpacing, computeLinePmRange, getCellSpacingPx, + normalizeBaselineShift, + resolveBaseFontSizeForVerticalText, shouldApplyJustify, SPACE_CHARS, } from '@superdoc/contracts'; @@ -7261,14 +7263,11 @@ const deriveBlockVersion = (block: FlowBlock): string => { return block.id; }; -const SUPERSCRIPT_SCALE = 0.65; const DEFAULT_SUPERSCRIPT_RAISE_RATIO = 0.33; const DEFAULT_SUBSCRIPT_LOWER_RATIO = 0.14; const hasVerticalPositioning = (run: TextRun): boolean => - (run.baselineShift != null && Number.isFinite(run.baselineShift)) || - run.vertAlign === 'superscript' || - run.vertAlign === 'subscript'; + normalizeBaselineShift(run.baselineShift) != null || run.vertAlign === 'superscript' || run.vertAlign === 'subscript'; const applyRunVerticalPositioning = (element: HTMLElement, run: TextRun): void => { // Vertically shifted runs should use a tight inline box. If they inherit the @@ -7278,19 +7277,20 @@ const applyRunVerticalPositioning = (element: HTMLElement, run: TextRun): void = element.style.lineHeight = '1'; } - if (run.baselineShift != null && Number.isFinite(run.baselineShift)) { - element.style.verticalAlign = `${run.baselineShift}pt`; + const explicitBaselineShift = normalizeBaselineShift(run.baselineShift); + if (explicitBaselineShift != null) { + element.style.verticalAlign = `${explicitBaselineShift}pt`; return; } if (run.vertAlign === 'superscript') { - const baseFontSize = run.fontSize / SUPERSCRIPT_SCALE; + const baseFontSize = resolveBaseFontSizeForVerticalText(run.fontSize, run); element.style.verticalAlign = `${baseFontSize * DEFAULT_SUPERSCRIPT_RAISE_RATIO}px`; return; } if (run.vertAlign === 'subscript') { - const baseFontSize = run.fontSize / SUPERSCRIPT_SCALE; + const baseFontSize = resolveBaseFontSizeForVerticalText(run.fontSize, run); element.style.verticalAlign = `${-(baseFontSize * DEFAULT_SUBSCRIPT_LOWER_RATIO)}px`; return; } diff --git a/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts b/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts index 5b758d1510..536ef9a44b 100644 --- a/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts +++ b/packages/layout-engine/painters/dom/src/text-style-rendering.test.ts @@ -463,6 +463,35 @@ describe('DomPainter text style CSS rendering', () => { expect(span?.style.verticalAlign).toBe('4pt'); }); + it('should treat zero baselineShift as identity and keep superscript rendering', () => { + const block = createParagraphBlock('para-va-5-zero', [ + { + text: '1st', + fontFamily: 'Arial', + fontSize: 10.4, + vertAlign: 'superscript' as const, + baselineShift: 0, + pmStart: 0, + pmEnd: 3, + }, + ]); + + const measure = createParagraphMeasure(); + const layout = createParagraphLayout('para-va-5-zero'); + + const painter = createDomPainter({ + blocks: [block], + measures: [measure], + }); + + painter.paint(layout, container); + + const span = container.querySelector('span'); + expect(span).toBeTruthy(); + expect(span?.style.lineHeight).toBe('1'); + expect(span?.style.verticalAlign).toBe('5.28px'); + }); + it('should apply negative baselineShift', () => { const block = createParagraphBlock('para-va-6', [ { diff --git a/packages/layout-engine/pm-adapter/src/attributes/paragraph.test.ts b/packages/layout-engine/pm-adapter/src/attributes/paragraph.test.ts index 7a87e9aa59..6cb18cd83e 100644 --- a/packages/layout-engine/pm-adapter/src/attributes/paragraph.test.ts +++ b/packages/layout-engine/pm-adapter/src/attributes/paragraph.test.ts @@ -337,6 +337,13 @@ describe('computeRunAttrs', () => { expect(result.fontSize).toBe(base.fontSize); }); + it('treats zero position as an identity value for superscript scaling', () => { + const base = computeRunAttrs({ fontSize: 24 } as never); + const result = computeRunAttrs({ fontSize: 24, vertAlign: 'superscript', position: 0 } as never); + expect(result.fontSize).toBeCloseTo(base.fontSize * 0.65); + expect(result.baselineShift).toBeUndefined(); + }); + it('converts position from half-points to points as baselineShift', () => { const result = computeRunAttrs({ position: 6 } as never); expect(result.baselineShift).toBe(3); @@ -346,4 +353,9 @@ describe('computeRunAttrs', () => { const result = computeRunAttrs({ fontSize: 24 } as never); expect(result.baselineShift).toBeUndefined(); }); + + it('does not set baselineShift for zero position', () => { + const result = computeRunAttrs({ position: 0 } as never); + expect(result.baselineShift).toBeUndefined(); + }); }); diff --git a/packages/layout-engine/pm-adapter/src/attributes/paragraph.ts b/packages/layout-engine/pm-adapter/src/attributes/paragraph.ts index a33f70e386..f9fb1f79ea 100644 --- a/packages/layout-engine/pm-adapter/src/attributes/paragraph.ts +++ b/packages/layout-engine/pm-adapter/src/attributes/paragraph.ts @@ -6,18 +6,20 @@ */ import { toCssFontFamily } from '@superdoc/font-utils'; -import type { - ParagraphAttrs, - ParagraphIndent, - DropCapDescriptor, - DropCapRun, - ParagraphFrame, +import { + normalizeBaselineShift, + scaleFontSizeForVerticalText, + type VerticalTextAlign, + type ParagraphAttrs, + type ParagraphIndent, + type DropCapDescriptor, + type DropCapRun, + type ParagraphFrame, } from '@superdoc/contracts'; import type { PMNode, ParagraphFont } from '../types.js'; import type { ResolvedRunProperties } from '@superdoc/word-layout'; import { computeWordParagraphLayout } from '@superdoc/word-layout'; import { pickNumber, twipsToPx, isFiniteNumber, ptToPx } from '../utilities.js'; -import { SUBSCRIPT_SUPERSCRIPT_SCALE } from '../constants.js'; import { normalizeAlignment, normalizeParagraphSpacing } from './spacing-indent.js'; import { normalizeOoxmlTabs } from './tabs.js'; import { normalizeParagraphBorders, normalizeParagraphShading } from './borders.js'; @@ -385,14 +387,12 @@ export const computeRunAttrs = ( fontFamily = runProps.fontFamily?.ascii || runProps.fontFamily?.hAnsi || runProps.fontFamily?.eastAsia || defaultFontFamily; } - const vertAlign = runProps.vertAlign as 'superscript' | 'subscript' | 'baseline' | undefined; - const hasPosition = runProps.position != null && Number.isFinite(runProps.position); - let fontSize = runProps.fontSize ? ptToPx(runProps.fontSize / 2)! : defaultFontSizePx; - - // Scale font size for superscript/subscript when no custom position override - if (!hasPosition && (vertAlign === 'superscript' || vertAlign === 'subscript')) { - fontSize *= SUBSCRIPT_SUPERSCRIPT_SCALE; - } + const vertAlign = runProps.vertAlign as VerticalTextAlign | undefined; + const baselineShift = normalizeBaselineShift( + runProps.position != null && Number.isFinite(runProps.position) ? runProps.position / 2 : undefined, + ); + const baseFontSize = runProps.fontSize ? ptToPx(runProps.fontSize / 2)! : defaultFontSizePx; + const fontSize = scaleFontSizeForVerticalText(baseFontSize, { vertAlign, baselineShift }); return { fontFamily: toCssFontFamily(fontFamily)!, @@ -415,6 +415,6 @@ export const computeRunAttrs = ( lang: runProps.lang?.val || undefined, vanish: runProps.vanish, vertAlign, - baselineShift: hasPosition ? runProps.position! / 2 : undefined, + baselineShift, }; }; diff --git a/packages/layout-engine/pm-adapter/src/constants.ts b/packages/layout-engine/pm-adapter/src/constants.ts index d706a3aa58..763034c022 100644 --- a/packages/layout-engine/pm-adapter/src/constants.ts +++ b/packages/layout-engine/pm-adapter/src/constants.ts @@ -5,12 +5,7 @@ import type { TextRun, TrackedChangeKind } from '@superdoc/contracts'; import type { HyperlinkConfig } from './types.js'; -/** - * Font size scaling factor for subscript and superscript text. - * Matches Microsoft Word's default rendering behavior for w:vertAlign - * when set to 'superscript' or 'subscript'. - */ -export const SUBSCRIPT_SUPERSCRIPT_SCALE = 0.65; +export { SUBSCRIPT_SUPERSCRIPT_SCALE } from '@superdoc/contracts'; /** * Unit conversion constants diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.test.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.test.ts index 9867dfaa4b..60f056ff56 100644 --- a/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.test.ts @@ -107,6 +107,29 @@ describe('buildReferenceMarkerRun', () => { expect(vi.mocked(textNodeToRun)).toHaveBeenCalledTimes(1); }); + it('treats a zero baselineShift as identity and still normalizes the marker', () => { + vi.mocked(textNodeToRun) + .mockReturnValueOnce({ + text: '1', + fontFamily: 'Calibri', + fontSize: 18, + baselineShift: 0, + vertAlign: 'superscript', + }) + .mockReturnValueOnce({ + text: '1', + fontFamily: 'Calibri', + fontSize: 24, + }); + + const run = buildReferenceMarkerRun('1', makeParams()); + + expect(run.vertAlign).toBe('superscript'); + expect(run.baselineShift).toBeUndefined(); + expect(run.fontSize).toBe(24 * SUBSCRIPT_SUPERSCRIPT_SCALE); + expect(vi.mocked(textNodeToRun)).toHaveBeenCalledTimes(2); + }); + it('preserves inherited styling from the original run context', () => { vi.mocked(textNodeToRun) .mockReturnValueOnce({ diff --git a/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.ts b/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.ts index f9ac87387c..c43db55344 100644 --- a/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.ts +++ b/packages/layout-engine/pm-adapter/src/converters/inline-converters/reference-marker.ts @@ -7,7 +7,7 @@ * preserving explicit custom positioning when the source document provides it. */ -import type { TextRun } from '@superdoc/contracts'; +import { hasExplicitBaselineShift, type TextRun } from '@superdoc/contracts'; import type { RunProperties } from '@superdoc/style-engine/ooxml'; import type { PMMark, PMNode } from '../../types.js'; import { SUBSCRIPT_SUPERSCRIPT_SCALE } from '../../constants.js'; @@ -73,9 +73,6 @@ const copyReferencePmPositions = (run: TextRun, params: InlineConverterParams): }; }; -const hasExplicitBaselineShift = (run: TextRun): boolean => - typeof run.baselineShift === 'number' && Number.isFinite(run.baselineShift); - const resolveReferenceBaseFontSize = ( runWithoutVerticalPositioning: TextRun, originalRun: TextRun, @@ -121,7 +118,7 @@ const buildReferenceRunWithoutVerticalPositioning = (displayText: string, params export function buildReferenceMarkerRun(displayText: string, params: InlineConverterParams): TextRun { const originalRun = buildOriginalReferenceRun(displayText, params); - if (hasExplicitBaselineShift(originalRun)) { + if (hasExplicitBaselineShift(originalRun.baselineShift)) { return copyReferencePmPositions(originalRun, params); } diff --git a/packages/layout-engine/pm-adapter/src/marks/application.test.ts b/packages/layout-engine/pm-adapter/src/marks/application.test.ts index ec7ebacabe..4f9bbda87a 100644 --- a/packages/layout-engine/pm-adapter/src/marks/application.test.ts +++ b/packages/layout-engine/pm-adapter/src/marks/application.test.ts @@ -873,6 +873,13 @@ describe('mark application', () => { applyTextStyleMark(run, { vertAlign: 'superscript', position: '3pt' }); expect(run.fontSize).toBe(16); }); + + it('treats zero position as an identity value for superscript scaling', () => { + const run: TextRun = { text: '1st', fontFamily: 'Arial', fontSize: 16 }; + applyTextStyleMark(run, { vertAlign: 'superscript', position: '0pt' }); + expect(run.fontSize).toBeCloseTo(16 * 0.65); + expect(run.baselineShift).toBeUndefined(); + }); }); describe('position / baselineShift', () => { @@ -888,6 +895,12 @@ describe('mark application', () => { expect(run.baselineShift).toBe(-1.5); }); + it('treats zero position as no explicit baseline shift', () => { + const run: TextRun = { text: 'text', fontFamily: 'Arial', fontSize: 16, baselineShift: 2 }; + applyTextStyleMark(run, { position: '0pt' }); + expect(run.baselineShift).toBeUndefined(); + }); + it('ignores non-numeric position', () => { const run: TextRun = { text: 'text', fontFamily: 'Arial', fontSize: 16 }; applyTextStyleMark(run, { position: 'invalid' }); diff --git a/packages/layout-engine/pm-adapter/src/marks/application.ts b/packages/layout-engine/pm-adapter/src/marks/application.ts index 931f6bf422..3c2ee5467f 100644 --- a/packages/layout-engine/pm-adapter/src/marks/application.ts +++ b/packages/layout-engine/pm-adapter/src/marks/application.ts @@ -8,10 +8,17 @@ * - Tracked changes (insert, delete, format) */ -import type { TextRun, TabRun, RunMark, TrackedChangeMeta, TrackedChangeKind } from '@superdoc/contracts'; +import { + normalizeBaselineShift, + scaleFontSizeForVerticalText, + type TextRun, + type TabRun, + type RunMark, + type TrackedChangeMeta, + type TrackedChangeKind, +} from '@superdoc/contracts'; import type { UnderlineStyle, PMMark, HyperlinkConfig, ThemeColorPalette } from '../types.js'; import { normalizeColor, isFiniteNumber, ptToPx } from '../utilities.js'; -import { SUBSCRIPT_SUPERSCRIPT_SCALE } from '../constants.js'; import { buildFlowRunLink, migrateLegacyLink } from './links.js'; import { sanitizeHref } from '@superdoc/url-validation'; import { resolveThemeColorValue } from './theme-color.js'; @@ -783,17 +790,21 @@ export const applyTextStyleMark = ( run.vertAlign = va; } } - // Custom baseline shift (position) — takes precedence over vertAlign for positioning + // Custom baseline shift (position) is explicit only when non-zero. + // A zero position is an identity value and should behave like "no shift". if (attrs.position != null && typeof attrs.position === 'string') { const parsed = parseFloat(attrs.position); if (Number.isFinite(parsed)) { - run.baselineShift = parsed; + const normalizedBaselineShift = normalizeBaselineShift(parsed); + if (normalizedBaselineShift == null) { + delete run.baselineShift; + } else { + run.baselineShift = normalizedBaselineShift; + } } } - // Scale font size for superscript/subscript when no custom position override - if (run.baselineShift == null && (run.vertAlign === 'superscript' || run.vertAlign === 'subscript')) { - run.fontSize *= SUBSCRIPT_SUPERSCRIPT_SCALE; - } + + run.fontSize = scaleFontSizeForVerticalText(run.fontSize, run); }; /** diff --git a/packages/super-editor/src/editors/v1/core/super-converter/styles.js b/packages/super-editor/src/editors/v1/core/super-converter/styles.js index 3cfcef1672..54305523ad 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/styles.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/styles.js @@ -13,6 +13,7 @@ import { } from '@converter/helpers.js'; import { SuperConverter } from '@converter/SuperConverter.js'; import { getUnderlineCssString } from '@extensions/linked-styles/underline-css.js'; +import { normalizeBaselineShift, SUBSCRIPT_SUPERSCRIPT_SCALE } from '@superdoc/contracts'; import { resolveDocxFontFamily, resolveRunProperties, @@ -31,14 +32,6 @@ const getToCssFontFamily = () => { return SuperConverter.toCssFontFamily; }; -/** - * Font size scaling factor for subscript and superscript text. - * This value (0.65 or 65%) matches Microsoft Word's default rendering behavior - * for vertical alignment (w:vertAlign) when set to 'superscript' or 'subscript'. - * Applied to the base font size to reduce text size for sub/superscripts. - */ -const SUBSCRIPT_SUPERSCRIPT_SCALE = 0.65; - /** * Encodes run property objects into mark definitions for the editor schema. * @param {Object} runProperties - Run properties extracted from DOCX. @@ -311,6 +304,10 @@ export function encodeCSSFromRPr(runProperties, docx) { let hasHighlightTag = false; let verticalAlignValue; let fontSizeOverride; + const normalizedPositionPoints = + runProperties.position != null && Number.isFinite(runProperties.position) + ? normalizeBaselineShift(halfPointToPoints(runProperties.position)) + : undefined; Object.keys(runProperties).forEach((key) => { const value = runProperties[key]; @@ -451,8 +448,8 @@ export function encodeCSSFromRPr(runProperties, docx) { break; } case 'vertAlign': { - // Skip if position is present - position takes precedence over vertAlign - if (runProperties.position != null && Number.isFinite(runProperties.position)) { + // Only non-zero positions override the default superscript/subscript offset. + if (normalizedPositionPoints != null) { break; } if (value === 'superscript' || value === 'subscript') { @@ -471,13 +468,9 @@ export function encodeCSSFromRPr(runProperties, docx) { break; } case 'position': { - if (value != null && Number.isFinite(value)) { - const points = halfPointToPoints(value); - if (Number.isFinite(points)) { - verticalAlignValue = `${points}pt`; - // Position takes precedence over vertAlign, so clear font-size override - fontSizeOverride = undefined; - } + if (normalizedPositionPoints != null) { + verticalAlignValue = `${normalizedPositionPoints}pt`; + fontSizeOverride = undefined; } break; } diff --git a/packages/super-editor/src/editors/v1/core/super-converter/styles.test.js b/packages/super-editor/src/editors/v1/core/super-converter/styles.test.js index e86bed59cf..e46295acf0 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/styles.test.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/styles.test.js @@ -302,10 +302,16 @@ describe('encodeCSSFromRPr - vertAlign/position edge cases', () => { it('handles zero position value', () => { const css = encodeCSSFromRPr({ position: 0 }, {}); - expect(css['vertical-align']).toBe('0pt'); + expect(css['vertical-align']).toBeUndefined(); + }); + + it('treats zero position as identity when combined with vertAlign', () => { + const css = encodeCSSFromRPr({ vertAlign: 'superscript', position: 0, fontSize: 20 }, {}); + expect(css['vertical-align']).toBe('super'); + expect(css['font-size']).toBe('6.5pt'); }); - it('position takes precedence over vertAlign when both are set', () => { + it('non-zero position takes precedence over vertAlign when both are set', () => { const css = encodeCSSFromRPr({ vertAlign: 'superscript', position: 4 }, {}); expect(css['vertical-align']).toBe('2pt'); expect(css['font-size']).toBeUndefined(); diff --git a/packages/super-editor/src/editors/v1/extensions/text-style/text-style.js b/packages/super-editor/src/editors/v1/extensions/text-style/text-style.js index f51ea4373e..01a89d03c3 100644 --- a/packages/super-editor/src/editors/v1/extensions/text-style/text-style.js +++ b/packages/super-editor/src/editors/v1/extensions/text-style/text-style.js @@ -1,8 +1,18 @@ // @ts-nocheck import { Mark } from '@core/Mark.js'; import { Attribute } from '@core/Attribute.js'; +import { normalizeBaselineShift } from '@superdoc/contracts'; import { annotationClass, annotationContentClass } from '../field-annotation/index.js'; +const hasExplicitPosition = (position) => { + if (typeof position !== 'string') { + return false; + } + + const parsed = parseFloat(position); + return normalizeBaselineShift(parsed) != null; +}; + /** * Configuration options for TextStyle * @typedef {Object} TextStyleOptions @@ -74,7 +84,8 @@ export const TextStyle = Mark.create({ /** * Vertical alignment for subscript/superscript text (DOCX w:vertAlign). * Standard values: 'superscript', 'subscript', 'baseline'. - * When both vertAlign and position are present, position takes precedence. + * Non-zero position values override the default superscript/subscript offset. + * A position of 0 is treated as an identity value. * Renders as CSS vertical-align with 65% font-size scaling for super/subscript. * @category Attribute * @param {string} [vertAlign] - Vertical alignment mode ('superscript' | 'subscript' | 'baseline') @@ -82,7 +93,7 @@ export const TextStyle = Mark.create({ vertAlign: { default: null, renderDOM: (attrs) => { - if (!attrs.vertAlign || attrs.position) return {}; + if (!attrs.vertAlign || hasExplicitPosition(attrs.position)) return {}; if (attrs.vertAlign === 'superscript') { return { style: 'vertical-align: super; font-size: 65%;' }; } @@ -106,7 +117,8 @@ export const TextStyle = Mark.create({ * Custom vertical position offset in points (DOCX w:position). * Numeric value specifying vertical offset (positive raises, negative lowers). * Format: '{number}pt' (e.g., '2pt', '-1.5pt'). - * Takes precedence over vertAlign when both are present. + * Non-zero position values override the default superscript/subscript offset. + * A position of 0 is treated as an identity value. * Renders as CSS vertical-align with the exact offset value. * @category Attribute * @param {string} [position] - Vertical position offset (e.g., '2pt', '-1pt') @@ -114,7 +126,7 @@ export const TextStyle = Mark.create({ position: { default: null, renderDOM: (attrs) => { - if (!attrs.position) return {}; + if (!hasExplicitPosition(attrs.position)) return {}; return { style: `vertical-align: ${attrs.position};` }; }, parseDOM: (el) => { diff --git a/packages/super-editor/src/editors/v1/extensions/types/mark-attributes.ts b/packages/super-editor/src/editors/v1/extensions/types/mark-attributes.ts index b1b4f88151..4ad6e3e311 100644 --- a/packages/super-editor/src/editors/v1/extensions/types/mark-attributes.ts +++ b/packages/super-editor/src/editors/v1/extensions/types/mark-attributes.ts @@ -133,7 +133,8 @@ export interface TextStyleAttrs { /** * Vertical alignment for subscript/superscript text (DOCX w:vertAlign). * Standard values: 'superscript', 'subscript', 'baseline'. - * When both vertAlign and position are present, position takes precedence. + * Non-zero position values override the default superscript/subscript offset. + * A position of 0 is treated as an identity value. * Renders as CSS vertical-align: super/sub with 65% font-size scaling. */ vertAlign?: 'superscript' | 'subscript' | 'baseline' | null; @@ -141,7 +142,7 @@ export interface TextStyleAttrs { * Custom vertical position offset in points (DOCX w:position). * Format: '{number}pt' where number is in points (e.g., '2pt', '-1.5pt'). * Positive values raise text, negative values lower text. - * Takes precedence over vertAlign when both are present. + * A position of 0 is treated as an identity value during rendering. * Renders as CSS vertical-align with the exact offset value. */ position?: string | null;