mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 14:00:37 +00:00
🐛 Guard blur-selection entity against stale text selections
Skip the PENPOT_SELECTION stamp when the selection is collapsed or points at blocks that no longer exist: blurring the editor rebuilds the EditorState with fresh block keys while a delayed on-change still carries the old keys, which crashed Draft.js reading getCharacterList of undefined and killed the workspace. Harden the sibling helpers (removeBlurSelectionEntity, cursorToEnd, select-all/end builders) the same way and add a vitest suite to the draft-js package covering stale, empty, and collapsed selections. Closes #11667 AI-assisted-by: muse-spark-1.3-contributor
This commit is contained in:
parent
655300e4d1
commit
a39b9766db
@ -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
|
||||
|
||||
|
||||
@ -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");
|
||||
|
||||
137
frontend/packages/draft-js/index.test.js
Normal file
137
frontend/packages/draft-js/index.test.js
Normal file
@ -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);
|
||||
});
|
||||
});
|
||||
@ -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"
|
||||
}
|
||||
}
|
||||
|
||||
8
frontend/packages/draft-js/vite.config.js
Normal file
8
frontend/packages/draft-js/vite.config.js
Normal file
@ -0,0 +1,8 @@
|
||||
import { defineConfig } from "vitest/config";
|
||||
|
||||
export default defineConfig({
|
||||
test: {
|
||||
include: ["index.test.js"],
|
||||
environment: "node",
|
||||
},
|
||||
});
|
||||
6
frontend/pnpm-lock.yaml
generated
6
frontend/pnpm-lock.yaml
generated
@ -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: {}
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user