[WC-3464] Rich text Tiptapeditor

[gjulivan]

Pull request type

---

Description

[github-actions]

AI Code Review

🚨 Blocked — high-severity issue (security, data loss, broken API) must be fixed

---

What was reviewed

File	Change
src/RichText.tsx	Simplification — removed old wrapper logic, delegates to EditorWrapper
src/RichText.xml	Added enableStatusBar, statusBarContent, styleDataFormat properties
src/components/Editor.tsx	Major rewrite — Quill → TipTap, useEditor hook, extension wiring
src/components/EditorWrapper.tsx	Rewrite — removed Quill integration, added status bar support
src/components/EditorContext.tsx	New — code-view reducer/context with useReducer
src/components/DynamicTableStyles.tsx	New — injects <style> tag for background-color class mode
src/components/DynamicTextColorStyles.tsx	New — injects <style> tag for text-color class mode
src/components/HighlightedCodeEditor.tsx	New — highlight.js + react-simple-code-editor for code view
src/components/LinkBubbleMenu.tsx	New — contextual bubble menu for link editing
src/components/toolbars/Toolbar.tsx	New toolbar tree: TipTap-native group/row rendering
src/components/toolbars/Toolbar.scss	New toolbar styles
src/components/LinkBubbleMenu.scss	New bubble menu styles
src/components/StatusBar.tsx	New — word/character count display
src/RichText.editorConfig.ts	Minor — hide statusBarContent when status bar disabled
src/__tests__/Indent.spec.ts	New — TipTap Indent extension unit tests
src/__tests__/LinkBubbleMenu.spec.ts	New — link bubble menu shouldShow/remove/edit unit tests
src/__tests__/RichText.spec.tsx	Extended — status bar snapshot tests
src/__tests__/customList.spec.ts	New — CustomListItem Quill-era format helper tests
src/__tests__/fonts.spec.ts	New — FontStyleAttributor / FontClassAttributor tests
src/__tests__/helpers.spec.ts	New — normalizeStyleAndClassAttribute helper tests
e2e/RichText.spec.js	68 new lines — class-mode, view-code, popup tests
package.json	TipTap v3 deps added, Quill removed
CHANGELOG.md	Unreleased entries for security fix, CSP class mode, empty-content fix

Skipped (out of scope): dist/, pnpm-lock.yaml, openspec/ docs-only files, binary .woff2/.mpk

---

Findings

🚨 High — CSS injection in dynamically generated <style> tags

Files: src/components/DynamicTableStyles.tsx line 46, src/components/DynamicTextColorStyles.tsx line 47
Problem: The raw color string from element.getAttribute("data-background-color") / element.getAttribute("data-text-color") is interpolated directly into a <style> element's textContent without sanitization. An attacker who can get arbitrary content into the editor (e.g., via pasted HTML) can inject a crafted data-background-color attribute value such as red} body{display:none or a url("javascript:...") expression, breaking out of the CSS rule and executing arbitrary CSS—or in older browsers, JavaScript.

The class-name part (sanitizedColor) is correctly sanitized via replace(/[^a-zA-Z0-9]/g, ""), but the value side of the CSS property is not.

Fix:

// Validate that the color is a safe CSS color value before use
const CSS_COLOR_SAFE = /^#[0-9a-fA-F]{3,8}$|^rgb\(\s*\d+\s*,\s*\d+\s*,\s*\d+\s*\)$|^rgba\(\s*\d+\s*,\s*\d+\s*,\s*\d+\s*,\s*[\d.]+\s*\)$/;

// In updateStyles():
if (color && CSS_COLOR_SAFE.test(color.trim())) {
    colorsInUse.add(color);
    // ... rest of logic
}

Or use the CSS CSS.supports API: if (CSS.supports("background-color", color)) — this delegates validation to the browser's CSS parser and is the most robust approach.

---

🔶 Medium — E2E tests reference Quill selectors after TipTap migration

File: e2e/RichText.spec.js lines 123–168 (new additions)
Problem: The newly added E2E tests still query Quill class names that no longer exist in the TipTap-based editor:

• .ql-editor (line 131) — TipTap uses .tiptap / .ProseMirror
• .ql-toolbar button.ql-view-code, .ql-toolbar button.ql-image (lines 144, 155)
• CSS class assertions: ql-color-, ql-bg-, ql-indent-, data-style-format="class" (lines 132–138)
• .widget-rich-text .widget-rich-text-modal-body (lines 145, 157) — the ModalDialog components were deleted in this PR

These tests will fail on the TipTap build and assert the wrong behaviour. Screenshots taken with Quill selectors against a TipTap DOM will produce empty matches.
Fix: Update selectors to match TipTap/new toolbar structure — e.g. .tiptap-editor for the editor content, .tiptap-toolbar for the toolbar, and the new dialog components' class names.

---

🔶 Medium — Null dereference in useEffect when editor is null

File: src/components/Editor.tsx line 282
Problem: useEditor(config, []) returns Editor | null. The useEffect at line 279 accesses editor.isFocused without a null guard. On the first render (before TipTap has initialised) and between mounts, editor is null, so this throws TypeError: Cannot read properties of null (reading 'isFocused').

// current — crashes when editor is null
useEffect(() => {
    if (!editor.isFocused) {

Fix:

useEffect(() => {
    if (!editor || !editor.isFocused) {
        return;
    }
    const newContent = editor.getHTML();
    if (newContent !== defaultValue) {
        editor.commands.setContent(defaultValue || "");
    }
}, [editor, defaultValue]);

---

🔶 Medium — Stale Mendix action closures in useEditor

File: src/components/Editor.tsx lines 254–265
Problem: useEditor is called with an empty dependency array ([]), so the onUpdate, onFocus, onBlur, and onCreate callbacks capture the initial render's props.onChange, props.onBlur, etc. as stale closures. Mendix ActionValue objects are replaced by reference on each render cycle (e.g., when the entity changes), so any action triggered after the first render will call the original stale reference — the action may have already been invalidated.

const editor = useEditor({ ..., onBlur: () => { executeAction(props.onBlur); ... } }, []);
//                                                                         ^^^^^^^^^ stale after re-render

Fix: Pass the action values via a stable ref so callbacks always read the latest value:

const propsRef = useRef(props);
useEffect(() => { propsRef.current = props; });

const editor = useEditor({
    onBlur: () => { executeAction(propsRef.current.onBlur); ... },
    ...
}, []);

---

⚠️ Low — page.waitForLoadState("networkidle") in E2E tests

File: e2e/RichText.spec.js lines 123, 130, 144
Note: Three new tests use page.waitForLoadState("networkidle") which is listed as a banned pattern in the E2E test guidelines. Replace with waitForMendixApp(page) or a web-first locator assertion:

// replace
await page.waitForLoadState("networkidle");
// with
await waitForMendixApp(page);
// or
await expect(page.locator(".mx-name-richText1")).toBeVisible();

---

⚠️ Low — <div onClick> for interactive buttons in ToolbarRowCode

File: src/components/toolbars/Toolbar.tsx lines 138, 143
Note: The Cancel/Save actions are <div> elements with onClick handlers. These are not keyboard-accessible (no Enter/Space activation, no focus, not announced as buttons by screen readers). Replace with <button type="button">:

<button type="button" className="mx-button btn btn-default" onClick={handleCancelCode}>
    {isDisabled ? "Close" : "Cancel"}
</button>
<button type="button" {...extraProps} className={classNames(...)} onClick={handleSaveCode} disabled={isDisabled}>
    Save
</button>

Using disabled on <button> is also cleaner than aria-disabled + manual class for this use case.

---

⚠️ Low — document.querySelectorAll scopes to entire page in dynamic style components

Files: src/components/DynamicTableStyles.tsx line 25, src/components/DynamicTextColorStyles.tsx line 27
Note: Querying document.querySelectorAll("[data-background-color]") picks up elements from every RichText widget instance on the page. With multiple widgets, one widget's updateStyles will write CSS rules for another widget's colors, causing cross-widget coupling. Scope the query to the editor's DOM node (editor.view.dom.querySelectorAll(...)) instead.

---

⚠️ Low — Inline styles for static design in HighlightedCodeEditor

File: src/components/HighlightedCodeEditor.tsx lines 47–57
Note: The style prop uses a JS object with static design values (fontFamily, fontSize, border, background, minHeight, etc.). Per the frontend guidelines, static design should use SCSS classes. Move these to a .scss file with a widget-prefixed class.

---

Positives

• The TipTap migration removes a large surface area (7 deleted components: ModalDialog/, CustomToolbars/, Toolbar.tsx, StickySentinel.tsx) and replaces it with a leaner architecture — very good scope reduction.
• EditorContext.tsx properly uses useReducer for the code-view state machine, making transitions explicit and testable.
• Indent.spec.ts is excellent — tests key invariants (margin-only, no structural nesting, accumulation, capping, keyboard routing) with a real TipTap editor instance rather than mocks.
• LinkBubbleMenu.spec.ts mirrors the component's shouldShow predicate directly in the test, making the logic verifiable without a React mount.
• executeAction is correctly used before all Mendix action calls; canExecute is not needed because executeAction guards it internally — consistent with the existing codebase pattern.
• The normalizeEmpty helper in EditorWrapper.tsx correctly treats <p></p> as empty to prevent empty-content false positives in required field validation.