diff --git a/packages/super-editor/src/core/extensions/keymap.js b/packages/super-editor/src/core/extensions/keymap.js index eaa5041b9a..2ad353bc08 100644 --- a/packages/super-editor/src/core/extensions/keymap.js +++ b/packages/super-editor/src/core/extensions/keymap.js @@ -74,7 +74,7 @@ export const Keymap = Extension.create({ 'Ctrl-Alt-Backspace': () => handleDelete(this.editor), 'Alt-Delete': () => handleDelete(this.editor), 'Alt-d': () => handleDelete(this.editor), - 'Ctrl-a': () => this.editor.commands.selectTextblockStart(), + 'Ctrl-a': () => this.editor.commands.selectAll(), 'Ctrl-e': () => this.editor.commands.selectTextblockEnd(), 'Ctrl-t': () => this.editor.commands.insertTabChar(), }; diff --git a/packages/super-editor/src/core/extensions/keymap.test.js b/packages/super-editor/src/core/extensions/keymap.test.js new file mode 100644 index 0000000000..74b9f229c2 --- /dev/null +++ b/packages/super-editor/src/core/extensions/keymap.test.js @@ -0,0 +1,47 @@ +import { describe, it, expect, vi } from 'vitest'; + +const setupKeymap = async ({ isMacOS, isIOS }) => { + vi.resetModules(); + vi.doMock('../utilities/isMacOS.js', () => ({ isMacOS: () => isMacOS })); + vi.doMock('../utilities/isIOS.js', () => ({ isIOS: () => isIOS })); + + const { Keymap } = await import('./keymap.js'); + const { getExtensionConfigField } = await import('../helpers/getExtensionConfigField.js'); + + const editor = { + commands: { + selectAll: vi.fn(), + selectTextblockStart: vi.fn(), + }, + }; + + const addShortcuts = getExtensionConfigField(Keymap, 'addShortcuts', { + name: Keymap.name, + editor, + }); + + const bindings = addShortcuts(); + return { bindings, editor }; +}; + +describe('Keymap extension', () => { + it('maps Ctrl-a to selectAll on macOS', async () => { + const { bindings, editor } = await setupKeymap({ isMacOS: true, isIOS: false }); + + expect(bindings['Ctrl-a']).toBeTypeOf('function'); + bindings['Ctrl-a'](); + + expect(editor.commands.selectAll).toHaveBeenCalledTimes(1); + expect(editor.commands.selectTextblockStart).not.toHaveBeenCalled(); + }); + + it('keeps Mod-a mapped to selectAll on non-mac platforms', async () => { + const { bindings, editor } = await setupKeymap({ isMacOS: false, isIOS: false }); + + expect(bindings['Mod-a']).toBeTypeOf('function'); + bindings['Mod-a'](); + + expect(editor.commands.selectAll).toHaveBeenCalledTimes(1); + expect(editor.commands.selectTextblockStart).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/super-editor/src/core/presentation-editor/dom/DomSelectionGeometry.ts b/packages/super-editor/src/core/presentation-editor/dom/DomSelectionGeometry.ts index 7bb398a389..d0beafebab 100644 --- a/packages/super-editor/src/core/presentation-editor/dom/DomSelectionGeometry.ts +++ b/packages/super-editor/src/core/presentation-editor/dom/DomSelectionGeometry.ts @@ -170,6 +170,21 @@ export function computeSelectionRectsFromDom( continue; } + const filterPageEntries = (entries: DomPositionIndexEntry[]) => + entries.filter((entry) => pageEl.contains(entry.el)); + + let pageEntries = filterPageEntries(sliceEntries); + if (pageEntries.length === 0 && !rebuiltOnce) { + options.rebuildDomPositionIndex(); + rebuiltOnce = true; + sliceEntries = options.domPositionIndex.findEntriesInRange(sliceFrom, sliceTo); + pageEntries = filterPageEntries(sliceEntries); + } + + if (pageEntries.length === 0) { + continue; + } + if (isVerbose) { debugLog( 'verbose', @@ -183,8 +198,33 @@ export function computeSelectionRectsFromDom( ); } - let startEntry = options.domPositionIndex.findEntryAtPosition(sliceFrom) ?? sliceEntries[0]!; - let endEntry = options.domPositionIndex.findEntryAtPosition(sliceTo) ?? sliceEntries[sliceEntries.length - 1]!; + const pickEntryForPos = (entries: DomPositionIndexEntry[], pos: number, fallbackIndex: number) => { + const direct = entries.find((entry) => pos >= entry.pmStart && pos <= entry.pmEnd); + if (!direct) { + const fallback = entries[fallbackIndex]!; + return fallback; + } + return direct; + }; + + let startEntry = pickEntryForPos(pageEntries, sliceFrom, 0); + let endEntry = pickEntryForPos(pageEntries, sliceTo, pageEntries.length - 1); + + if ((!startEntry?.el?.isConnected || !endEntry?.el?.isConnected) && !rebuiltOnce) { + options.rebuildDomPositionIndex(); + rebuiltOnce = true; + sliceEntries = options.domPositionIndex.findEntriesInRange(sliceFrom, sliceTo); + pageEntries = filterPageEntries(sliceEntries); + if (pageEntries.length === 0) { + continue; + } + startEntry = pickEntryForPos(pageEntries, sliceFrom, 0); + endEntry = pickEntryForPos(pageEntries, sliceTo, pageEntries.length - 1); + } + + if (!startEntry?.el?.isConnected || !endEntry?.el?.isConnected) { + continue; + } if (isVerbose) { debugLog( @@ -199,61 +239,6 @@ export function computeSelectionRectsFromDom( ); } - // If the index is stale (virtualization mount/unmount), rebuild once and retry. - let startContained = pageEl.contains(startEntry.el); - let endContained = pageEl.contains(endEntry.el); - if (!startContained || !endContained) { - if (isVerbose) { - debugLog( - 'verbose', - `DOM selection rects: boundary containment ${JSON.stringify({ - pageIndex, - sliceFrom, - sliceTo, - startContained, - endContained, - })}`, - ); - } - if (!rebuiltOnce) { - options.rebuildDomPositionIndex(); - rebuiltOnce = true; - sliceEntries = options.domPositionIndex.findEntriesInRange(sliceFrom, sliceTo); - if (sliceEntries.length === 0) continue; - startEntry = options.domPositionIndex.findEntryAtPosition(sliceFrom) ?? sliceEntries[0]!; - endEntry = options.domPositionIndex.findEntryAtPosition(sliceTo) ?? sliceEntries[sliceEntries.length - 1]!; - startContained = pageEl.contains(startEntry.el); - endContained = pageEl.contains(endEntry.el); - if (isVerbose) { - debugLog( - 'verbose', - `DOM selection rects: boundary containment after rebuild ${JSON.stringify({ - pageIndex, - sliceFrom, - sliceTo, - startContained, - endContained, - start: entryDebugInfo(startEntry), - end: entryDebugInfo(endEntry), - })}`, - ); - } - } - if (!startContained || !endContained) { - debugLog( - 'warn', - `DOM selection rects: stale index after rebuild ${JSON.stringify({ - pageIndex, - sliceFrom, - sliceTo, - start: entryDebugInfo(startEntry), - end: entryDebugInfo(endEntry), - })}`, - ); - return null; - } - } - const doc = pageEl.ownerDocument ?? document; const range = doc.createRange(); try { @@ -280,7 +265,7 @@ export function computeSelectionRectsFromDom( } let missingEntries: DomPositionIndexEntry[] | null = null; if (typeof range.intersectsNode === 'function') { - for (const entry of sliceEntries) { + for (const entry of pageEntries) { try { if (!range.intersectsNode(entry.el)) { missingEntries ??= []; @@ -304,7 +289,7 @@ export function computeSelectionRectsFromDom( })}`, ); } - rawRects = collectClientRectsByLine(doc, sliceEntries, sliceFrom, sliceTo); + rawRects = collectClientRectsByLine(doc, pageEntries, sliceFrom, sliceTo); if (dumpRects) { debugLog( 'verbose', diff --git a/packages/super-editor/src/core/presentation-editor/tests/DomSelectionGeometry.test.ts b/packages/super-editor/src/core/presentation-editor/tests/DomSelectionGeometry.test.ts index 6f2ab9cebc..fdd9774e23 100644 --- a/packages/super-editor/src/core/presentation-editor/tests/DomSelectionGeometry.test.ts +++ b/packages/super-editor/src/core/presentation-editor/tests/DomSelectionGeometry.test.ts @@ -477,12 +477,19 @@ describe('computeSelectionRectsFromDom', () => { beforeEach(() => { painterHost = document.createElement('div'); + // Append to document body so elements have isConnected === true + document.body.appendChild(painterHost); domPositionIndex = new DomPositionIndex(); rebuildDomPositionIndex = vi.fn(() => { domPositionIndex.rebuild(painterHost); }); }); + afterEach(() => { + // Clean up after each test + painterHost.remove(); + }); + /** * Helper to create a minimal Layout object for testing */ @@ -664,6 +671,52 @@ describe('computeSelectionRectsFromDom', () => { document.createRange = originalCreateRange; }); + + it('handles duplicate PM ranges across pages (repeated table headers)', () => { + painterHost.innerHTML = ` +
+
+ header row +
+
+
+
+ header row (repeat) +
+
+ `; + + const layout = createMockLayout([ + { pmStart: 1, pmEnd: 10 }, + { pmStart: 1, pmEnd: 10 }, + ]); + domPositionIndex.rebuild(painterHost); + + const pages = Array.from(painterHost.querySelectorAll('.superdoc-page')) as HTMLElement[]; + pages[0]!.getBoundingClientRect = vi.fn(() => createRect(0, 0, 612, 792)); + pages[1]!.getBoundingClientRect = vi.fn(() => createRect(0, 808, 612, 792)); + + const mockRange = { + setStart: vi.fn(), + setEnd: vi.fn(), + getClientRects: vi.fn(() => [createRect(10, 20, 100, 16)]), + } as unknown as Range; + + const originalCreateRange = document.createRange; + document.createRange = vi.fn(() => mockRange); + + const options = createOptions(layout); + const rects = computeSelectionRectsFromDom(options, 1, 10); + + expect(rects).not.toBe(null); + expect(rects!.length).toBeGreaterThan(0); + + const pageIndices = new Set(rects!.map((r) => r.pageIndex)); + expect(pageIndices.has(0)).toBe(true); + expect(pageIndices.has(1)).toBe(true); + + document.createRange = originalCreateRange; + }); }); describe('index rebuild behavior', () => { @@ -726,6 +779,153 @@ describe('computeSelectionRectsFromDom', () => { expect(rects).toBe(null); }); + + it('rebuilds index when page entries are empty after initial filtering', () => { + painterHost.innerHTML = ` +
+
+ text +
+
+ `; + + const layout = createMockLayout([{ pmStart: 1, pmEnd: 10 }]); + + // Build index but then clear the painterHost to simulate stale entries + domPositionIndex.rebuild(painterHost); + + // Re-add content so rebuild finds it + const pageEl = painterHost.querySelector('.superdoc-page') as HTMLElement; + pageEl.getBoundingClientRect = vi.fn(() => createRect(0, 0, 612, 792)); + + const mockRange = { + setStart: vi.fn(), + setEnd: vi.fn(), + getClientRects: vi.fn(() => [createRect(10, 20, 50, 16)]), + } as unknown as Range; + + const originalCreateRange = document.createRange; + document.createRange = vi.fn(() => mockRange); + + const options = createOptions(layout); + computeSelectionRectsFromDom(options, 1, 10); + + // The function should work without errors + document.createRange = originalCreateRange; + }); + + it('skips page when entries remain disconnected after rebuild', () => { + painterHost.innerHTML = ` +
+
+ text +
+
+ `; + + const layout = createMockLayout([{ pmStart: 1, pmEnd: 10 }]); + domPositionIndex.rebuild(painterHost); + + const pageEl = painterHost.querySelector('.superdoc-page') as HTMLElement; + const spanEl = painterHost.querySelector('span') as HTMLElement; + pageEl.getBoundingClientRect = vi.fn(() => createRect(0, 0, 612, 792)); + + // Mock isConnected to return false, simulating disconnected element + Object.defineProperty(spanEl, 'isConnected', { + get: () => false, + configurable: true, + }); + + const options = createOptions(layout); + const rects = computeSelectionRectsFromDom(options, 1, 10); + + // Should return empty array (page skipped due to disconnected elements) + expect(rects).toEqual([]); + }); + }); + + describe('page-scoped entry selection', () => { + it('uses fallback entry when position does not match any entry directly', () => { + painterHost.innerHTML = ` +
+
+ middle text +
+
+ `; + + // Layout has range 1-20, but DOM only has 5-15 + const layout = createMockLayout([{ pmStart: 1, pmEnd: 20 }]); + domPositionIndex.rebuild(painterHost); + + const pageEl = painterHost.querySelector('.superdoc-page') as HTMLElement; + pageEl.getBoundingClientRect = vi.fn(() => createRect(0, 0, 612, 792)); + + const mockRange = { + setStart: vi.fn(), + setEnd: vi.fn(), + getClientRects: vi.fn(() => [createRect(10, 20, 100, 16)]), + } as unknown as Range; + + const originalCreateRange = document.createRange; + document.createRange = vi.fn(() => mockRange); + + const options = createOptions(layout); + // Request range 1-20, but entries only cover 5-15 + // pickEntryForPos should use fallback for positions outside entry range + const rects = computeSelectionRectsFromDom(options, 1, 20); + + expect(rects).not.toBe(null); + expect(rects!.length).toBeGreaterThan(0); + + document.createRange = originalCreateRange; + }); + + it('filters entries to only those contained in current page', () => { + // Two pages with different PM ranges, entries indexed globally + painterHost.innerHTML = ` +
+
+ page 0 text +
+
+
+
+ page 1 text +
+
+ `; + + const layout = createMockLayout([ + { pmStart: 1, pmEnd: 10 }, + { pmStart: 11, pmEnd: 20 }, + ]); + domPositionIndex.rebuild(painterHost); + + const pages = Array.from(painterHost.querySelectorAll('.superdoc-page')) as HTMLElement[]; + pages[0]!.getBoundingClientRect = vi.fn(() => createRect(0, 0, 612, 792)); + pages[1]!.getBoundingClientRect = vi.fn(() => createRect(0, 808, 612, 792)); + + const mockRange = { + setStart: vi.fn(), + setEnd: vi.fn(), + getClientRects: vi.fn(() => [createRect(10, 20, 100, 16)]), + } as unknown as Range; + + const originalCreateRange = document.createRange; + document.createRange = vi.fn(() => mockRange); + + const options = createOptions(layout); + const rects = computeSelectionRectsFromDom(options, 1, 20); + + expect(rects).not.toBe(null); + // Should have rects from both pages, each filtered to its own entries + const pageIndices = new Set(rects!.map((r) => r.pageIndex)); + expect(pageIndices.has(0)).toBe(true); + expect(pageIndices.has(1)).toBe(true); + + document.createRange = originalCreateRange; + }); }); describe('virtualized pages (not mounted)', () => { @@ -1174,12 +1374,19 @@ describe('computeDomCaretPageLocal', () => { beforeEach(() => { painterHost = document.createElement('div'); + // Append to document body so elements have isConnected === true + document.body.appendChild(painterHost); domPositionIndex = new DomPositionIndex(); rebuildDomPositionIndex = vi.fn(() => { domPositionIndex.rebuild(painterHost); }); }); + afterEach(() => { + // Clean up after each test + painterHost.remove(); + }); + function createCaretOptions(): ComputeDomCaretPageLocalOptions { return { painterHost,