From 1e1a5b0daa793595f4fda3b7d7555e13d683aa5b Mon Sep 17 00:00:00 2001 From: Popa Serban Alexandru Date: Wed, 5 Aug 2026 15:40:52 +0300 Subject: [PATCH 1/2] refactor(super-editor): extract shared caret line-anchoring predicate Caret paths each decided independently whether an element's own box or its enclosing line box is the caret box. A tab span and an empty SDT placeholder are painted with a box that is deliberately not the line box, so reading the element's own rect puts the caret off its row. Move that rule into dom-observer/CaretLineAnchoring.ts and route the body caret path through it, so a new caret path gets the rule by construction rather than by remembering a comment. No behavior change. --- .../dom-observer/CaretLineAnchoring.test.ts | 81 +++++++++++++++++++ .../v1/dom-observer/CaretLineAnchoring.ts | 31 +++++++ .../v1/dom-observer/DomSelectionGeometry.ts | 11 +-- 3 files changed, 115 insertions(+), 8 deletions(-) create mode 100644 packages/super-editor/src/editors/v1/dom-observer/CaretLineAnchoring.test.ts create mode 100644 packages/super-editor/src/editors/v1/dom-observer/CaretLineAnchoring.ts diff --git a/packages/super-editor/src/editors/v1/dom-observer/CaretLineAnchoring.test.ts b/packages/super-editor/src/editors/v1/dom-observer/CaretLineAnchoring.test.ts new file mode 100644 index 0000000000..ad236e7750 --- /dev/null +++ b/packages/super-editor/src/editors/v1/dom-observer/CaretLineAnchoring.test.ts @@ -0,0 +1,81 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { isEmptySdtPlaceholder, isLineAnchoredCaretElement, resolveCaretLineBox } from './CaretLineAnchoring.js'; + +function createRect(top: number, height: number): DOMRect { + return { + top, + height, + bottom: top + height, + left: 0, + right: 0, + width: 0, + x: 0, + y: top, + toJSON: () => ({}), + } as DOMRect; +} + +function paintInLine(className: string, lineRect = createRect(200, 18)): HTMLElement { + const line = document.createElement('div'); + line.className = 'superdoc-line'; + vi.spyOn(line, 'getBoundingClientRect').mockReturnValue(lineRect); + + const el = document.createElement('span'); + el.className = className; + line.appendChild(el); + document.body.appendChild(line); + return el; +} + +afterEach(() => { + document.body.innerHTML = ''; + vi.restoreAllMocks(); +}); + +describe('isEmptySdtPlaceholder', () => { + it.each([ + 'superdoc-empty-sdt-placeholder', + 'superdoc-empty-inline-sdt-placeholder', + 'superdoc-empty-block-sdt-placeholder', + ])('matches %s', (className) => { + expect(isEmptySdtPlaceholder(paintInLine(className))).toBe(true); + }); + + it('does not match a tab', () => { + expect(isEmptySdtPlaceholder(paintInLine('superdoc-tab'))).toBe(false); + }); +}); + +describe('isLineAnchoredCaretElement', () => { + it('matches tabs and empty SDT placeholders', () => { + expect(isLineAnchoredCaretElement(paintInLine('superdoc-tab'))).toBe(true); + expect(isLineAnchoredCaretElement(paintInLine('superdoc-empty-inline-sdt-placeholder'))).toBe(true); + }); + + it('does not match elements whose own box is the caret box', () => { + expect(isLineAnchoredCaretElement(paintInLine('superdoc-inline-image'))).toBe(false); + }); +}); + +describe('resolveCaretLineBox', () => { + it('returns the enclosing line box for a tab', () => { + expect(resolveCaretLineBox(paintInLine('superdoc-tab'))).toMatchObject({ top: 200, height: 18 }); + }); + + it('returns null for elements that are not line-anchored', () => { + expect(resolveCaretLineBox(paintInLine('superdoc-inline-image'))).toBeNull(); + }); + + it('returns null when the tab has no enclosing line', () => { + const orphan = document.createElement('span'); + orphan.className = 'superdoc-tab'; + document.body.appendChild(orphan); + + expect(resolveCaretLineBox(orphan)).toBeNull(); + }); + + it('returns null for a degenerate line box so callers keep the element box', () => { + expect(resolveCaretLineBox(paintInLine('superdoc-tab', createRect(200, 0)))).toBeNull(); + }); +}); diff --git a/packages/super-editor/src/editors/v1/dom-observer/CaretLineAnchoring.ts b/packages/super-editor/src/editors/v1/dom-observer/CaretLineAnchoring.ts new file mode 100644 index 0000000000..ee7f1d0981 --- /dev/null +++ b/packages/super-editor/src/editors/v1/dom-observer/CaretLineAnchoring.ts @@ -0,0 +1,31 @@ +import { DOM_CLASS_NAMES } from '@superdoc/dom-contract'; + +const EMPTY_SDT_PLACEHOLDER_CLASSES = [ + 'superdoc-empty-sdt-placeholder', + 'superdoc-empty-inline-sdt-placeholder', + 'superdoc-empty-block-sdt-placeholder', +]; + +/** Placeholder painted for an SDT with no content. */ +export function isEmptySdtPlaceholder(el: HTMLElement): boolean { + return EMPTY_SDT_PLACEHOLDER_CLASSES.some((className) => el.classList.contains(className)); +} + +/** + * AIDEV-NOTE: A tab span is painted `vertical-align: bottom` (SD-3330) inside a `font-size: 0` + * line, so its own box starts below the line top. Every caret path must route through this — + * the header/footer path missed the body-only fix in #3677 and kept rendering the caret low. + */ +export function isLineAnchoredCaretElement(el: HTMLElement): boolean { + return isEmptySdtPlaceholder(el) || el.classList.contains('superdoc-tab'); +} + +/** The line box a caret should use, or null when the element's own box is correct. */ +export function resolveCaretLineBox(el: HTMLElement): DOMRect | null { + if (!isLineAnchoredCaretElement(el)) return null; + + const lineRect = el.closest(`.${DOM_CLASS_NAMES.LINE}`)?.getBoundingClientRect(); + if (!lineRect || !Number.isFinite(lineRect.top) || lineRect.height <= 0) return null; + + return lineRect; +} diff --git a/packages/super-editor/src/editors/v1/dom-observer/DomSelectionGeometry.ts b/packages/super-editor/src/editors/v1/dom-observer/DomSelectionGeometry.ts index d2d5a266a6..4248be610d 100644 --- a/packages/super-editor/src/editors/v1/dom-observer/DomSelectionGeometry.ts +++ b/packages/super-editor/src/editors/v1/dom-observer/DomSelectionGeometry.ts @@ -1,6 +1,7 @@ import type { Layout } from '@superdoc/contracts'; import { DOM_CLASS_NAMES } from '@superdoc/dom-contract'; +import { isEmptySdtPlaceholder, resolveCaretLineBox } from './CaretLineAnchoring.js'; import type { DomPositionIndex, DomPositionIndexEntry } from './DomPositionIndex.js'; import { debugLog, getSelectionDebugConfig } from '../core/presentation-editor/selection/SelectionDebug.js'; @@ -647,14 +648,8 @@ export function computeDomCaretPageLocal( // For non-text elements (images, math), position caret at the right edge // when pos matches pmEnd (cursor after the element) - const isEmptySdtPlaceholder = - targetEl.classList.contains('superdoc-empty-sdt-placeholder') || - targetEl.classList.contains('superdoc-empty-inline-sdt-placeholder') || - targetEl.classList.contains('superdoc-empty-block-sdt-placeholder'); - const atEnd = isEmptySdtPlaceholder ? pos > entry.pmEnd : pos >= entry.pmEnd; - const useLineTopForY = isEmptySdtPlaceholder || targetEl.classList.contains('superdoc-tab'); - const lineEl = useLineTopForY ? (targetEl.closest('.superdoc-line') as HTMLElement | null) : null; - const yRect = lineEl?.getBoundingClientRect() ?? elRect; + const atEnd = isEmptySdtPlaceholder(targetEl) ? pos > entry.pmEnd : pos >= entry.pmEnd; + const yRect = resolveCaretLineBox(targetEl) ?? elRect; return { pageIndex: Number(page.dataset.pageIndex ?? '0'), x: ((atEnd ? elRect.right : elRect.left) - pageRect.left) / zoom, From 5db4b35a5989ee09c1d10715a36f18ce046a0a39 Mon Sep 17 00:00:00 2001 From: Popa Serban Alexandru Date: Wed, 5 Aug 2026 15:41:16 +0300 Subject: [PATCH 2/2] fix(super-editor): anchor header/footer caret to the line box after a tab Typing in a header or footer and pressing Tab rendered the caret about 5.5px below its own line, hanging past the bottom of the row. Typing any character snapped it back, so the insertion point was always correct - only the caret's painted position between the Tab and the next keystroke was wrong. A painted tab span is deliberately height: line.lineHeight with vertical-align: bottom (SD-3330) inside a .superdoc-line carrying font-size: 0, so its own border box starts below the line top. computeCaretRect resolves through #computeVisibleSurfaceCaretRect, which falls back to the element's own rect for entries with no child text node. A tab span is painted empty, so it took that branch and handed the tab's box to the caret as both y and height. Same symptom as #3507, which #3677 fixed for body text. That fix only touched DomSelectionGeometry, so the header/footer path never got it. Refs #3507, #3677 --- .../HeaderFooterSessionManager.ts | 6 +- .../tests/HeaderFooterSessionManager.test.ts | 83 +++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts index 43ff2361a5..627b6e860f 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts @@ -62,6 +62,7 @@ import { type HeaderFooterConstraints, } from '@superdoc/layout-bridge'; import { selectionToRects } from '@superdoc/layout-bridge'; +import { resolveCaretLineBox } from '../../../dom-observer/CaretLineAnchoring.js'; import { deduplicateOverlappingRects } from '../../../dom-observer/DomSelectionGeometry.js'; import { resolveSectionProjections } from '../../../document-api-adapters/helpers/sections-resolver.js'; import { computeCaretLayoutRectGeometry as computeCaretLayoutRectGeometryFromHelper } from '../selection/CaretGeometry.js'; @@ -2236,12 +2237,13 @@ export class HeaderFooterSessionManager { } const localX = (pos <= entry.pmStart ? elementRect.left : elementRect.right) - pageRect.left; + const yRect = resolveCaretLineBox(entry.el) ?? elementRect; return { pageIndex: context.region.pageIndex, x: localX / zoom, - y: context.region.pageIndex * bodyPageHeight + (elementRect.top - pageRect.top) / zoom, + y: context.region.pageIndex * bodyPageHeight + (yRect.top - pageRect.top) / zoom, width: 1, - height: Math.max(1, elementRect.height / zoom), + height: Math.max(1, yRect.height / zoom), }; } diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts index 8303da579b..233bd82f6b 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts @@ -2668,4 +2668,87 @@ describe('HeaderFooterSessionManager', () => { expect(duringUpdate.storyLayouts.headers[0]!.resolvedLayout!.pages[0]!.items[0]!.blockId).toBe('old-block'); }); }); + + describe('computeCaretRect — text-less surface entries', () => { + // Page rect is the origin, so page-local coords equal client coords and the + // only interesting term is `pageIndex * bodyPageHeight` = 1 * 800. + const PAGE_RECT = createRect(0, 0, 816, 1056); + const LINE_RECT = createRect(100, 200, 600, 18); + // A painted tab is `vertical-align: bottom` inside a `font-size: 0` line, so its + // own box starts below the line top. That 6px is the caret offset users saw. + const TAB_RECT = createRect(140, 206, 20, 18); + + async function setupSurface(entryEl: HTMLElement, { wrapInLine = true } = {}): Promise { + await setupWithZoom(1); + + const pageElement = painterHost.querySelector('[data-page-index="1"]')!; + vi.spyOn(pageElement, 'getBoundingClientRect').mockReturnValue(PAGE_RECT); + + const surface = document.createElement('div'); + surface.className = 'superdoc-page-header'; + pageElement.appendChild(surface); + + vi.spyOn(entryEl, 'getBoundingClientRect').mockReturnValue(TAB_RECT); + + if (wrapInLine) { + const line = document.createElement('div'); + line.className = 'superdoc-line'; + vi.spyOn(line, 'getBoundingClientRect').mockReturnValue(LINE_RECT); + line.appendChild(entryEl); + surface.appendChild(line); + } else { + surface.appendChild(entryEl); + } + } + + function createEntryEl(className: string): HTMLElement { + const el = document.createElement('span'); + if (className) { + el.className = className; + } + el.dataset.pmStart = '7'; + el.dataset.pmEnd = '8'; + return el; + } + + it('anchors the caret to the line box for a tab entry, not the bottom-aligned tab box', async () => { + await setupSurface(createEntryEl('superdoc-tab')); + + expect(manager.computeCaretRect(9)).toEqual({ + pageIndex: 1, + // x still comes from the tab box: pos is past pmStart, so the caret sits at its right edge. + x: TAB_RECT.right, + y: 800 + LINE_RECT.top, + width: 1, + height: LINE_RECT.height, + }); + }); + + it('anchors the caret to the line box for an empty SDT placeholder', async () => { + await setupSurface(createEntryEl('superdoc-empty-inline-sdt-placeholder')); + + expect(manager.computeCaretRect(9)).toMatchObject({ + y: 800 + LINE_RECT.top, + height: LINE_RECT.height, + }); + }); + + it('keeps the element box for text-less entries that are not line-anchored', async () => { + await setupSurface(createEntryEl('superdoc-inline-image')); + + expect(manager.computeCaretRect(9)).toMatchObject({ + y: 800 + TAB_RECT.top, + height: TAB_RECT.height, + }); + }); + + it('falls back to the element box when a tab entry has no enclosing line', async () => { + await setupSurface(createEntryEl('superdoc-tab'), { wrapInLine: false }); + + expect(manager.computeCaretRect(9)).toMatchObject({ + y: 800 + TAB_RECT.top, + height: TAB_RECT.height, + }); + }); + }); });