diff --git a/packages/super-editor/src/core/helpers/getMarksFromSelection.js b/packages/super-editor/src/core/helpers/getMarksFromSelection.js index b492da1fc9..fad706cf68 100644 --- a/packages/super-editor/src/core/helpers/getMarksFromSelection.js +++ b/packages/super-editor/src/core/helpers/getMarksFromSelection.js @@ -62,7 +62,7 @@ export function getFormattingStateAtPos(state, pos, editor, options = {}) { const resolvedRunProperties = resolvedFromSelection?.resolvedRunProperties ?? inlineRunProperties; const styleRunProperties = resolvedFromSelection?.styleRunProperties ?? null; const resolvedMarksFromProperties = createMarksFromRunProperties(state, resolvedRunProperties, editor); - resolvedMarks.push(...(resolvedMarksFromProperties.length ? resolvedMarksFromProperties : inlineMarks)); + resolvedMarks.push(...mergeResolvedMarksWithInlineFallback(resolvedMarksFromProperties, inlineMarks)); if (storedMarks && includeCursorMarksWithStoredMarks) { resolvedMarks.push(...cursorMarks); } @@ -99,16 +99,28 @@ function aggregateFormattingSegments(state, editor, segments) { const resolvedRunProperties = intersectRunProperties(segments.map((segment) => segment.resolvedRunProperties)); const inlineRunProperties = intersectRunProperties(segments.map((segment) => segment.inlineRunProperties)); const styleRunProperties = intersectRunProperties(segments.map((segment) => segment.styleRunProperties)); + const resolvedMarks = createMarksFromRunProperties(state, resolvedRunProperties, editor); + const inlineMarks = createMarksFromRunProperties(state, inlineRunProperties, editor); return { - resolvedMarks: createMarksFromRunProperties(state, resolvedRunProperties, editor), - inlineMarks: createMarksFromRunProperties(state, inlineRunProperties, editor), + resolvedMarks: mergeResolvedMarksWithInlineFallback(resolvedMarks, inlineMarks), + inlineMarks, resolvedRunProperties, inlineRunProperties, styleRunProperties, }; } +function mergeResolvedMarksWithInlineFallback(resolvedMarks, inlineMarks) { + if (!resolvedMarks.length) return inlineMarks; + if (!inlineMarks.length) return resolvedMarks; + + const resolvedMarkNames = new Set(resolvedMarks.map((mark) => mark.type.name)); + const missingInlineMarks = inlineMarks.filter((mark) => !resolvedMarkNames.has(mark.type.name)); + + return [...resolvedMarks, ...missingInlineMarks]; +} + function intersectRunProperties(runPropertiesList) { const filtered = runPropertiesList.filter((props) => props && typeof props === 'object'); if (filtered.length === 0) return null; diff --git a/packages/super-editor/src/core/helpers/getMarksFromSelection.resolved-fallback.test.js b/packages/super-editor/src/core/helpers/getMarksFromSelection.resolved-fallback.test.js new file mode 100644 index 0000000000..c8f5af7949 --- /dev/null +++ b/packages/super-editor/src/core/helpers/getMarksFromSelection.resolved-fallback.test.js @@ -0,0 +1,71 @@ +import { describe, expect, it, vi } from 'vitest'; +import { EditorState, TextSelection } from 'prosemirror-state'; +import { Schema } from 'prosemirror-model'; + +const resolveRunProperties = vi.fn(() => ({ bold: true })); + +vi.mock('@superdoc/style-engine/ooxml', () => ({ + resolveRunProperties, +})); + +vi.mock('@extensions/paragraph/resolvedPropertiesCache.js', () => ({ + calculateResolvedParagraphProperties: vi.fn(() => ({})), +})); + +describe('getSelectionFormattingState resolved mark fallback', () => { + it('preserves inline highlight when resolved marks omit it', async () => { + const { getSelectionFormattingState } = await import('./getMarksFromSelection.js'); + + const schema = new Schema({ + nodes: { + doc: { content: 'paragraph+' }, + paragraph: { + content: 'inline*', + group: 'block', + toDOM() { + return ['p', 0]; + }, + }, + run: { + content: 'text*', + group: 'inline', + inline: true, + attrs: { runProperties: { default: null } }, + toDOM() { + return ['span', 0]; + }, + }, + text: { group: 'inline' }, + }, + marks: { + bold: { + attrs: { value: { default: true } }, + toDOM() { + return ['strong', 0]; + }, + }, + highlight: { + attrs: { color: { default: null } }, + toDOM() { + return ['mark', 0]; + }, + }, + }, + }); + + const doc = schema.node('doc', null, [ + schema.node('paragraph', null, [ + schema.node('run', { runProperties: { highlight: { 'w:val': '#ECCF35' } } }, [schema.text('Hello')]), + ]), + ]); + const baseState = EditorState.create({ schema, doc }); + const state = baseState.apply(baseState.tr.setSelection(TextSelection.create(doc, 2, 7))); + + const result = getSelectionFormattingState(state, { converter: { convertedXml: {} } }); + + expect(resolveRunProperties).toHaveBeenCalled(); + expect(result.inlineMarks).toContainEqual(expect.objectContaining({ attrs: { color: '#ECCF35' } })); + expect(result.resolvedMarks).toContainEqual(expect.objectContaining({ attrs: { value: true } })); + expect(result.resolvedMarks).toContainEqual(expect.objectContaining({ attrs: { color: '#ECCF35' } })); + }); +}); diff --git a/packages/super-editor/src/core/helpers/getMarksFromSelection.test.js b/packages/super-editor/src/core/helpers/getMarksFromSelection.test.js index ba2eb3dba3..f4a10a6f8c 100644 --- a/packages/super-editor/src/core/helpers/getMarksFromSelection.test.js +++ b/packages/super-editor/src/core/helpers/getMarksFromSelection.test.js @@ -202,6 +202,52 @@ describe('getMarksFromSelection', () => { expect(result.inlineRunProperties).toEqual({ styleId: 'Heading1Char' }); }); + it('reconstructs highlight marks from hash-prefixed runProperties values', () => { + const runSchema = new Schema({ + nodes: { + doc: { content: 'paragraph+' }, + paragraph: { + content: 'inline*', + group: 'block', + toDOM() { + return ['p', 0]; + }, + }, + run: { + content: 'text*', + group: 'inline', + inline: true, + attrs: { runProperties: { default: null } }, + toDOM() { + return ['span', 0]; + }, + }, + text: { group: 'inline' }, + }, + marks: { + highlight: { + attrs: { color: { default: null } }, + toDOM() { + return ['mark', 0]; + }, + }, + }, + }); + const testDoc = runSchema.node('doc', null, [ + runSchema.node('paragraph', null, [ + runSchema.node('run', { runProperties: { highlight: { 'w:val': '#ECCF35' } } }, [runSchema.text('Hello')]), + ]), + ]); + const state = EditorState.create({ schema: runSchema, doc: testDoc }); + const cursorState = state.apply(state.tr.setSelection(TextSelection.create(testDoc, 3))); + + const result = getSelectionFormattingState(cursorState); + + expect(result.inlineRunProperties).toEqual({ highlight: { 'w:val': '#ECCF35' } }); + expect(result.inlineMarks).toContainEqual(expect.objectContaining({ attrs: { color: '#ECCF35' } })); + expect(result.resolvedMarks).toContainEqual(expect.objectContaining({ attrs: { color: '#ECCF35' } })); + }); + it('falls back to cursor marks when the surrounding run has no explicit runProperties', () => { const runSchema = new Schema({ nodes: { diff --git a/packages/super-editor/src/core/super-converter/styles.js b/packages/super-editor/src/core/super-converter/styles.js index 01ebc076b8..3cfcef1672 100644 --- a/packages/super-editor/src/core/super-converter/styles.js +++ b/packages/super-editor/src/core/super-converter/styles.js @@ -7,8 +7,10 @@ import { twipsToLines, eighthPointsToPixels, linesToTwips, + isValidHexColor, + getHexColorFromDocxSystem, + normalizeHexColor, } from '@converter/helpers.js'; -import { isValidHexColor, getHexColorFromDocxSystem } from '@converter/helpers'; import { SuperConverter } from '@converter/SuperConverter.js'; import { getUnderlineCssString } from '@extensions/linked-styles/underline-css.js'; import { @@ -670,11 +672,35 @@ function getFontFamilyValue(attributes, docx) { * @returns {string|null} Hex color string, 'transparent', or null when unsupported. */ function getHighLightValue(attributes) { - const fill = attributes['w:fill']; - if (fill && fill !== 'auto') return `#${fill}`; - if (attributes?.['w:val'] === 'none') return 'transparent'; - if (isValidHexColor(attributes?.['w:val'])) return `#${attributes['w:val']}`; - return getHexColorFromDocxSystem(attributes?.['w:val']) || null; + const fill = normalizeHighlightHex(attributes?.['w:fill']); + if (fill) return `#${fill}`; + + const value = attributes?.['w:val']; + if (value === 'none') return 'transparent'; + + const normalizedValue = normalizeHighlightHex(value); + if (normalizedValue) return `#${normalizedValue}`; + + return getHexColorFromDocxSystem(value) || null; +} + +/** + * Normalize a highlight token to a 6-digit hex string without a leading hash. + * Returns null for non-hex values such as DOCX system color keywords. + * + * @param {unknown} rawValue + * @returns {string|null} + */ +function normalizeHighlightHex(rawValue) { + if (typeof rawValue !== 'string') return null; + + const trimmedValue = rawValue.trim(); + if (!trimmedValue || trimmedValue.toLowerCase() === 'auto') return null; + + const normalizedValue = normalizeHexColor(trimmedValue); + if (!normalizedValue || !isValidHexColor(normalizedValue)) return null; + + return normalizedValue; } /** diff --git a/packages/super-editor/src/core/super-converter/styles.test.js b/packages/super-editor/src/core/super-converter/styles.test.js index 5055753258..e86bed59cf 100644 --- a/packages/super-editor/src/core/super-converter/styles.test.js +++ b/packages/super-editor/src/core/super-converter/styles.test.js @@ -51,6 +51,15 @@ describe('encodeMarksFromRPr', () => { }); }); + it('should encode highlight from a hash-prefixed w:val', () => { + const rPr = { highlight: { 'w:val': '#ECCF35' } }; + const marks = encodeMarksFromRPr(rPr, {}); + expect(marks).toContainEqual({ + type: 'highlight', + attrs: { color: '#ECCF35' }, + }); + }); + it('should encode highlight from w:shd', () => { const rPr = { shading: { fill: 'FFA500' } }; const marks = encodeMarksFromRPr(rPr, {}); diff --git a/tests/behavior/tests/comments/comment-highlight-noop-focus.spec.ts b/tests/behavior/tests/comments/comment-highlight-noop-focus.spec.ts deleted file mode 100644 index 7608c4dd79..0000000000 --- a/tests/behavior/tests/comments/comment-highlight-noop-focus.spec.ts +++ /dev/null @@ -1,177 +0,0 @@ -import type { Page } from '@playwright/test'; -import { expect, test } from '../../fixtures/superdoc.js'; -import { addCommentByText, assertDocumentApiReady } from '../../helpers/document-api.js'; - -test.use({ config: { toolbar: 'full', comments: 'on' } }); - -async function typeParagraphs( - superdoc: { type: (text: string) => Promise; newLine: () => Promise; waitForStable: () => Promise }, - paragraphs: string[], -) { - for (const [index, paragraph] of paragraphs.entries()) { - await superdoc.type(paragraph); - if (index < paragraphs.length - 1) { - await superdoc.newLine(); - } - } - - await superdoc.waitForStable(); -} - -async function dispatchRetargetedRepeatClickOnCommentHighlight(page: Page, highlightedText: string) { - await page.waitForFunction( - (textMatch) => { - const highlights = Array.from(document.querySelectorAll('.superdoc-comment-highlight')); - const highlight = highlights.find((candidate) => { - if (!(candidate.textContent ?? '').includes(textMatch)) { - return false; - } - - const rect = candidate.getBoundingClientRect(); - return rect.width > 0 && rect.height > 0; - }); - - if (!highlight) { - return null; - } - - const rect = highlight.getBoundingClientRect(); - const viewport = document.querySelector('.presentation-editor__viewport'); - - if (!viewport) { - return null; - } - - const eventCoordinates = { - clientX: rect.left + rect.width / 2, - clientY: rect.top + rect.height / 2, - screenX: rect.left + rect.width / 2, - screenY: rect.top + rect.height / 2, - }; - const pointerBase = { - bubbles: true, - cancelable: true, - composed: true, - pointerId: 1, - pointerType: 'mouse', - isPrimary: true, - }; - - highlight.dispatchEvent( - new PointerEvent('pointerdown', { - ...pointerBase, - ...eventCoordinates, - button: 0, - buttons: 1, - }), - ); - - viewport.dispatchEvent( - new PointerEvent('pointerup', { - ...pointerBase, - ...eventCoordinates, - button: 0, - buttons: 0, - }), - ); - - viewport.dispatchEvent( - new MouseEvent('click', { - bubbles: true, - cancelable: true, - composed: true, - ...eventCoordinates, - button: 0, - buttons: 0, - }), - ); - - return true; - }, - highlightedText, - { timeout: 10_000 }, - ); -} - -test('clicking the active editor comment again keeps the same thread active', async ({ superdoc, browserName }) => { - test.skip(browserName !== 'chromium', 'This regression asserts Chromium editor-click behavior.'); - - const getActiveCommentState = () => - superdoc.page.evaluate(() => { - const editor = (window as any).editor; - const store = (window as any).superdoc?.commentsStore; - const activeDialogIds = Array.from(document.querySelectorAll('.comments-dialog.is-active')).map((dialog) => - dialog.closest('.comment-placeholder')?.getAttribute('data-comment-id'), - ); - - return { - activeComment: store?.activeComment ?? null, - selection: editor - ? { - from: editor.state.selection.from, - to: editor.state.selection.to, - empty: editor.state.selection.empty, - } - : null, - activeDialogIds, - }; - }); - - await assertDocumentApiReady(superdoc.page); - - await typeParagraphs(superdoc, [ - 'Top line with AlphaTarget.', - 'Filler line 1.', - 'Filler line 2.', - 'Filler line 3.', - 'Filler line 4.', - 'Bottom line with BetaTarget.', - ]); - - await addCommentByText(superdoc.page, { - pattern: 'AlphaTarget', - text: 'Alpha comment body', - }); - await addCommentByText(superdoc.page, { - pattern: 'BetaTarget', - text: 'Beta comment body', - }); - await superdoc.waitForStable(); - - const alphaThreadId = await superdoc.page - .locator('.comment-placeholder', { hasText: 'Alpha comment body' }) - .first() - .getAttribute('data-comment-id'); - if (!alphaThreadId) { - throw new Error('Expected the alpha comment placeholder to expose a thread id'); - } - - await superdoc.page.evaluate((activeCommentId) => { - const superdocInstance = (window as any).superdoc; - const editor = (window as any).editor; - - superdocInstance?.commentsStore?.setActiveComment?.(superdocInstance, activeCommentId); - editor?.commands?.setCursorById?.(activeCommentId, { preferredActiveThreadId: activeCommentId }); - }, alphaThreadId); - await superdoc.waitForStable(); - - await expect - .poll(() => getActiveCommentState(superdoc.page)) - .toMatchObject({ - activeComment: alphaThreadId, - selection: { empty: true }, - activeDialogIds: [alphaThreadId], - }); - - await superdoc.page.waitForTimeout(600); - await dispatchRetargetedRepeatClickOnCommentHighlight(superdoc.page, 'AlphaTarget'); - await superdoc.waitForStable(); - - await expect - .poll(() => getActiveCommentState(superdoc.page)) - .toMatchObject({ - activeComment: alphaThreadId, - selection: { empty: true }, - activeDialogIds: [alphaThreadId], - }); -}); diff --git a/tests/behavior/tests/comments/edit-comment-text.spec.ts b/tests/behavior/tests/comments/edit-comment-text.spec.ts index d7577390b0..4eeaaa9a25 100644 --- a/tests/behavior/tests/comments/edit-comment-text.spec.ts +++ b/tests/behavior/tests/comments/edit-comment-text.spec.ts @@ -27,7 +27,7 @@ test('editing a comment updates its text', async ({ superdoc }) => { await superdoc.waitForStable(); // The comment should now be in edit mode - const editInput = dialog.locator('.comment-editing .superdoc-field'); + const editInput = dialog.locator('.reply-expanded .superdoc-field'); await expect(editInput).toBeVisible({ timeout: 5_000 }); // Select all text in the edit input, then type the replacement @@ -37,7 +37,7 @@ test('editing a comment updates its text', async ({ superdoc }) => { await superdoc.waitForStable(); // Click Update - await dialog.locator('.comment-editing .sd-button.primary', { hasText: 'Update' }).click(); + await dialog.locator('.reply-expanded .sd-button.primary', { hasText: 'Update' }).click(); await superdoc.waitForStable(); // After update the dialog loses is-active; verify the text changed via the visible sidebar dialog diff --git a/tests/behavior/tests/comments/undo-tracked-insert-removes-sidebar.spec.ts b/tests/behavior/tests/comments/undo-tracked-insert-removes-sidebar.spec.ts index aae9175675..e58a39eb5d 100644 --- a/tests/behavior/tests/comments/undo-tracked-insert-removes-sidebar.spec.ts +++ b/tests/behavior/tests/comments/undo-tracked-insert-removes-sidebar.spec.ts @@ -1,9 +1,17 @@ -import { test, expect } from '../../fixtures/superdoc.js'; +import { test, expect, type SuperDocFixture } from '../../fixtures/superdoc.js'; import { getDocumentText, listTrackChanges } from '../../helpers/document-api.js'; import { activateCommentDialog } from '../../helpers/comments.js'; test.use({ config: { toolbar: 'full', comments: 'panel', trackChanges: true } }); +async function historyUndo(superdoc: Pick) { + return superdoc.page.evaluate(() => (window as any).editor.doc.history.undo()); +} + +async function historyRedo(superdoc: Pick) { + return superdoc.page.evaluate(() => (window as any).editor.doc.history.redo()); +} + test('undo tracked insertion removes suggestion bubble and sidebar entry', async ({ superdoc }) => { const sidebar = superdoc.page.locator('.superdoc__right-sidebar'); const sidebarTrackedChange = sidebar.locator('.tracked-change-text'); @@ -18,9 +26,10 @@ test('undo tracked insertion removes suggestion bubble and sidebar entry', async await expect(sidebar).toBeVisible(); await expect.poll(async () => sidebarTrackedChange.count()).toBeGreaterThan(0); - await superdoc.undo(); + const result = await historyUndo(superdoc); await superdoc.waitForStable(); + expect(result.noop).toBe(false); await expect.poll(async () => (await listTrackChanges(superdoc.page)).total).toBe(0); await expect(sidebarTrackedChange).toHaveCount(0); await expect( @@ -44,9 +53,10 @@ test('redo restores tracked insertion bubble and sidebar entry after undo', asyn await expect.poll(async () => sidebarTrackedChange.count()).toBeGreaterThan(0); await expect(await activateCommentDialog(superdoc, 'Tracked insertion')).toBeVisible(); - await superdoc.undo(); + const undoResult = await historyUndo(superdoc); await superdoc.waitForStable(); + expect(undoResult.noop).toBe(false); await expect.poll(async () => (await listTrackChanges(superdoc.page)).total).toBe(0); await expect(sidebarTrackedChange).toHaveCount(0); await expect( @@ -55,9 +65,10 @@ test('redo restores tracked insertion bubble and sidebar entry after undo', asyn }), ).toHaveCount(0); - await superdoc.redo(); + const redoResult = await historyRedo(superdoc); await superdoc.waitForStable(); + expect(redoResult.noop).toBe(false); await expect.poll(async () => (await listTrackChanges(superdoc.page)).total).toBeGreaterThanOrEqual(1); await expect.poll(async () => sidebarTrackedChange.count()).toBeGreaterThan(0); await expect(await activateCommentDialog(superdoc, 'Tracked insertion')).toBeVisible();