diff --git a/.serena/memories/frontend/ui-packages-text-editor-workflow.md b/.serena/memories/frontend/ui-packages-text-editor-workflow.md index 24fdd9383c..9b78a994d8 100644 --- a/.serena/memories/frontend/ui-packages-text-editor-workflow.md +++ b/.serena/memories/frontend/ui-packages-text-editor-workflow.md @@ -9,6 +9,8 @@ - `frontend/text-editor` builds `@penpot/text-editor` from `src/editor/TextEditor.js`. It is a Vite JS package, not CLJS, and has its own Vitest/browser-test setup. - The text editor consumes render-wasm artifacts copied from `frontend/resources/public/js` into `frontend/text-editor/src/wasm`. Use `pnpm run wasm:update` after rebuilding `render-wasm` if tests or local dev use stale WASM files. - Other packages under `frontend/packages/` such as `tokenscript`, `draft-js`, and `mousetrap` are workspace dependencies used by the frontend app; do not assume their runtime behavior lives in CLJS namespaces. +- `frontend/packages/draft-js` (Draft.js wrapper) validates selections before every `Modifier.applyEntity` call and returns the input state unchanged on stale/invalid/collapsed selections: blurring the text editor rebuilds `EditorState` with fresh block keys while a delayed `on-change` still carries old keys, which otherwise crashes Draft.js (`getCharacterList` of undefined) and kills the workspace. +- `frontend/packages/draft-js` has its own `vite.config.js` pinning `include: ["index.test.js"]`: without it, `vitest run` from that dir is hijacked by the frontend-root `vite.config.js` storybook project and reports "No test files found". Run via `pnpm --filter @penpot/draft-js test`; it also runs inside `frontend/scripts/test` (CI `Unit Tests` step), right after `pnpm install` and before the wasm build. ## Commands diff --git a/frontend/packages/draft-js/index.js b/frontend/packages/draft-js/index.js index 5372d7ee40..1b19e7f53f 100644 --- a/frontend/packages/draft-js/index.js +++ b/frontend/packages/draft-js/index.js @@ -29,6 +29,31 @@ function isDefined(v) { return v !== undefined && v !== null; } +// A selection can outlive the content it points to: blurring the editor +// rebuilds the EditorState with fresh block keys while a delayed on-change +// still carries the old keys. Applying an entity with such a stale selection +// crashes inside Draft.js (`getCharacterList` of undefined), taking down the +// whole workspace. Validate before every `Modifier.applyEntity` call and +// skip silently: the blur highlight is cosmetic. +function isSelectionInContent(content, selection) { + if (!content || !selection) return false; + const blockMap = content.getBlockMap(); + if (!blockMap || blockMap.isEmpty()) return false; + + const startKey = selection.getStartKey(); + const endKey = selection.getEndKey(); + const startBlock = blockMap.get(startKey); + const endBlock = blockMap.get(endKey); + if (!startBlock || !endBlock) return false; + + const startOffset = selection.getStartOffset(); + const endOffset = selection.getEndOffset(); + if (startOffset < 0 || endOffset < 0) return false; + if (startOffset > startBlock.getLength()) return false; + if (endOffset > endBlock.getLength()) return false; + return true; +} + function mergeBlockData(block, newData) { if (!block) return undefined; let data = block.getData(); @@ -70,8 +95,10 @@ export function createDecorator(type, component) { function getSelectAllSelection(state) { const content = state.getCurrentContent(); - const firstBlock = content.getBlockMap().first(); - const lastBlock = content.getBlockMap().last(); + const blockMap = content.getBlockMap(); + if (!blockMap || blockMap.isEmpty()) return null; + const firstBlock = blockMap.first(); + const lastBlock = blockMap.last(); return new SelectionState({ "anchorKey": firstBlock.getKey(), @@ -83,7 +110,9 @@ function getSelectAllSelection(state) { function getCursorInEndPosition(state) { const content = state.getCurrentContent(); - const lastBlock = content.getBlockMap().last(); + const blockMap = content.getBlockMap(); + if (!blockMap || blockMap.isEmpty()) return null; + const lastBlock = blockMap.last(); return new SelectionState({ "anchorKey": lastBlock.getKey(), @@ -94,7 +123,9 @@ function getCursorInEndPosition(state) { } export function selectAll(state) { - return EditorState.forceSelection(state, getSelectAllSelection(state)); + const selection = getSelectAllSelection(state); + if (!selection) return state; + return EditorState.forceSelection(state, selection); } function modifySelectedBlocks(contentState, selectionState, operation) { @@ -192,22 +223,35 @@ export function splitBlockPreservingData(state) { } export function addBlurSelectionEntity(state) { - let content = state.getCurrentContent(state); const selection = state.getSelection(); + if (selection.isCollapsed()) return state; - content = content.createEntity("PENPOT_SELECTION", "MUTABLE"); - const entityKey = content.getLastCreatedEntityKey(); + let content = state.getCurrentContent(); + if (!isSelectionInContent(content, selection)) return state; - content = Modifier.applyEntity(content, selection, entityKey); + try { + content = content.createEntity("PENPOT_SELECTION", "MUTABLE"); + const entityKey = content.getLastCreatedEntityKey(); + content = Modifier.applyEntity(content, selection, entityKey); + } catch (_) { + return state; + } return EditorState.push(state, content, "apply-entity"); } export function removeBlurSelectionEntity(state) { const selectionAll = getSelectAllSelection(state); + if (!selectionAll) return state; const selection = state.getSelection(); let content = state.getCurrentContent(); - content = Modifier.applyEntity(content, selectionAll, null); + if (!isSelectionInContent(content, selection)) return state; + + try { + content = Modifier.applyEntity(content, selectionAll, null); + } catch (_) { + return state; + } state = EditorState.push(state, content, "apply-entity"); state = EditorState.forceSelection(state, selection); @@ -264,10 +308,16 @@ export function removeInlineStylePrefix(contentState, selectionState, stylePrefi export function cursorToEnd(state) { const newSelection = getCursorInEndPosition(state); - const selection = state.getSelection(); + if (!newSelection) return state; let content = state.getCurrentContent(); - content = Modifier.applyEntity(content, newSelection, null); + if (!isSelectionInContent(content, newSelection)) return state; + + try { + content = Modifier.applyEntity(content, newSelection, null); + } catch (_) { + return state; + } state = EditorState.forceSelection(state, newSelection); state = EditorState.push(state, content, "apply-entity"); diff --git a/frontend/packages/draft-js/index.test.js b/frontend/packages/draft-js/index.test.js new file mode 100644 index 0000000000..0ebacd1f46 --- /dev/null +++ b/frontend/packages/draft-js/index.test.js @@ -0,0 +1,137 @@ +/** + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + * + * Copyright (c) KALEIDOS INC Sucursal en España SL + */ +import { describe, test, expect } from "vitest"; +import { + EditorState, + SelectionState, + addBlurSelectionEntity, + convertFromRaw, + createEditorState, + cursorToEnd, + removeBlurSelectionEntity, +} from "./index.js"; + +/* @vitest-environment node */ + +function makeContent() { + return convertFromRaw({ + blocks: [ + { + key: "first", + text: "Hello", + type: "unstyled", + depth: 0, + inlineStyleRanges: [], + entityRanges: [], + data: {}, + }, + { + key: "second", + text: "World", + type: "unstyled", + depth: 0, + inlineStyleRanges: [], + entityRanges: [], + data: {}, + }, + ], + entityMap: {}, + }); +} + +function makeState() { + return createEditorState(makeContent(), null); +} + +function makeSelection(anchorKey, anchorOffset, focusKey, focusOffset) { + return new SelectionState({ + anchorKey, + anchorOffset, + focusKey, + focusOffset, + isBackward: false, + hasFocus: true, + }); +} + +function withSelection(state, selection) { + return EditorState.forceSelection(state, selection); +} + +describe("addBlurSelectionEntity", () => { + test("does not throw on stale selection keys and returns state unchanged", () => { + const state = makeState(); + const stale = withSelection( + state, + makeSelection("missing-start", 0, "missing-end", 3), + ); + expect(() => addBlurSelectionEntity(stale)).not.toThrow(); + expect(addBlurSelectionEntity(stale)).toBe(stale); + }); + + test("skips collapsed selections and returns state unchanged", () => { + const state = makeState(); + const collapsed = withSelection( + state, + makeSelection("first", 2, "first", 2), + ); + expect(addBlurSelectionEntity(collapsed)).toBe(collapsed); + }); + + test("applies PENPOT_SELECTION entity on valid non-collapsed selections", () => { + const state = makeState(); + const selected = withSelection( + state, + makeSelection("first", 0, "first", 5), + ); + const result = addBlurSelectionEntity(selected); + const block = result.getCurrentContent().getBlockForKey("first"); + expect(block.getEntityAt(0)).not.toBeNull(); + expect(block.getEntityAt(4)).not.toBeNull(); + expect(block.getEntityAt(0)).toBe(block.getEntityAt(4)); + }); +}); + +describe("removeBlurSelectionEntity", () => { + test("does not throw on stale selection keys and returns state unchanged", () => { + const state = makeState(); + const stale = withSelection( + state, + makeSelection("missing-start", 0, "missing-end", 3), + ); + expect(() => removeBlurSelectionEntity(stale)).not.toThrow(); + expect(removeBlurSelectionEntity(stale)).toBe(stale); + }); + + test("clears entities and restores the user selection", () => { + const state = makeState(); + const selected = withSelection( + state, + makeSelection("first", 1, "second", 2), + ); + const blurred = addBlurSelectionEntity(selected); + const result = removeBlurSelectionEntity(blurred); + expect( + result.getCurrentContent().getBlockForKey("first").getEntityAt(1), + ).toBeNull(); + expect(result.getSelection().getAnchorKey()).toBe( + blurred.getSelection().getAnchorKey(), + ); + }); +}); + +describe("cursorToEnd", () => { + test("moves the selection to the end of the last block", () => { + const state = makeState(); + const result = cursorToEnd(state); + const selection = result.getSelection(); + expect(selection.getAnchorKey()).toBe("second"); + expect(selection.getAnchorOffset()).toBe(5); + expect(selection.isCollapsed()).toBe(true); + }); +}); diff --git a/frontend/packages/draft-js/package.json b/frontend/packages/draft-js/package.json index aa187debb2..a80002b067 100644 --- a/frontend/packages/draft-js/package.json +++ b/frontend/packages/draft-js/package.json @@ -10,11 +10,16 @@ "dependencies": { "draft-js": "penpot/draft-js.git#c58ebd9429a6359d72a88cff87e078aaf6fe285d" }, + "scripts": { + "test": "vitest run" + }, "peerDependencies": { "react": ">=0.17.0", "react-dom": ">=0.17.0" }, "devDependencies": { - "esbuild": "^0.28.1" + "esbuild": "^0.28.1", + "jsdom": "^30.0.1", + "vitest": "^4.1.10" } } diff --git a/frontend/packages/draft-js/vite.config.js b/frontend/packages/draft-js/vite.config.js new file mode 100644 index 0000000000..13bb10c319 --- /dev/null +++ b/frontend/packages/draft-js/vite.config.js @@ -0,0 +1,8 @@ +import { defineConfig } from "vitest/config"; + +export default defineConfig({ + test: { + include: ["index.test.js"], + environment: "node", + }, +}); diff --git a/frontend/pnpm-lock.yaml b/frontend/pnpm-lock.yaml index 3f045541a8..9e07661e70 100644 --- a/frontend/pnpm-lock.yaml +++ b/frontend/pnpm-lock.yaml @@ -442,6 +442,12 @@ importers: esbuild: specifier: ^0.28.1 version: 0.28.2 + jsdom: + specifier: ^30.0.1 + version: 30.0.1(canvas@3.2.3) + vitest: + specifier: ^4.1.10 + version: 4.1.11(@types/node@26.4.0)(@vitest/browser-playwright@4.1.11)(@vitest/coverage-v8@4.1.11)(@vitest/ui@4.1.11)(jsdom@30.0.1(canvas@3.2.3))(vite@8.2.2(@types/node@26.4.0)(esbuild@0.28.2)(sass-embedded@1.103.1)(sass@1.103.1)) packages/mousetrap: {} diff --git a/frontend/scripts/test b/frontend/scripts/test index 6d822b9542..1b98028a46 100755 --- a/frontend/scripts/test +++ b/frontend/scripts/test @@ -3,6 +3,9 @@ set -ex pnpm install; +# Fast JS package unit tests (no wasm/browser needed) +pnpm --filter @penpot/draft-js test; + # Build render wasm binary pushd ../render-wasm; ./build