';
+ expect(purifyHtml(input)).toBe(input);
+ });
+
+ test('script tags still do not survive alongside a style attribute', () => {
+ const out = purifyHtml('
ok
');
+ expect(out).not.toContain(' {
+ const out = purifyHtml('
x
');
+ expect(out).not.toContain('onclick');
+ expect(out).toContain('style="color:#ff0000"');
+ });
+
+ test('javascript: URLs still do not survive on an element that also carries style', () => {
+ const out = purifyHtml('x');
+ expect(out).not.toContain('javascript:');
+ expect(out).toContain('style="color:#ff0000"');
+ });
+});
+
describe('purifyHtml iframe sandboxing (M-6)', () => {
test('forces a restrictive sandbox attribute onto every iframe', () => {
const out = purifyHtml('');
diff --git a/craft/src/components/basic/HtmlBlock.toHtml.test.ts b/craft/src/components/basic/HtmlBlock.toHtml.test.ts
index dafc090..48e0e18 100644
--- a/craft/src/components/basic/HtmlBlock.toHtml.test.ts
+++ b/craft/src/components/basic/HtmlBlock.toHtml.test.ts
@@ -33,3 +33,16 @@ test('toHtml never emits the style prop (the other half of the render/export con
expect(out.html).not.toContain('background');
expect(out.html).not.toContain('40px');
});
+
+describe('HtmlBlock.toHtml markup path (C1 review finding)', () => {
+ test('a style attribute inside `code` (e.g. from the toolbar colour picker) reaches exported output', () => {
+ const { html } = toHtml({ code: '
';
+ const { html } = toHtml({ code }, '');
+ expect(html).toBe(code);
+ });
+});
diff --git a/craft/src/components/basic/HtmlBlock.tsx b/craft/src/components/basic/HtmlBlock.tsx
index 367d7f2..befbeca 100644
--- a/craft/src/components/basic/HtmlBlock.tsx
+++ b/craft/src/components/basic/HtmlBlock.tsx
@@ -19,10 +19,17 @@ const PURIFY_CONFIG = {
'blockquote','code','pre',
'img','figure','figcaption',
'iframe',
+ // Tables: pasted content commonly includes these; dropping them
+ // silently ate customer-pasted tables (see C1 review finding).
+ 'table','thead','tbody','tfoot','tr','td','th','caption','colgroup','col',
],
+ // NOTE: supplying ALLOWED_ATTR replaces DOMPurify's own default attribute
+ // allowlist rather than extending it, so anything the product needs
+ // (style, id, ...) must be listed explicitly here even though DOMPurify
+ // would allow it by default.
ALLOWED_ATTR: [
'href','src','alt','title','target','rel',
- 'width','height','class',
+ 'width','height','class','id','style',
'allowfullscreen','allow','frameborder',
'sandbox','referrerpolicy',
],
diff --git a/craft/src/hooks/useWhpApi.orphan-repair-wiring.test.tsx b/craft/src/hooks/useWhpApi.orphan-repair-wiring.test.tsx
index e89c1a7..180b238 100644
--- a/craft/src/hooks/useWhpApi.orphan-repair-wiring.test.tsx
+++ b/craft/src/hooks/useWhpApi.orphan-repair-wiring.test.tsx
@@ -3,7 +3,7 @@ import React from 'react';
import { createRoot, Root } from 'react-dom/client';
import { act } from 'react-dom/test-utils';
import { EditorConfigProvider } from '../state/EditorConfigContext';
-import { PageProvider, usePages } from '../state/PageContext';
+import { EMPTY_CANVAS, PageProvider, usePages } from '../state/PageContext';
import { SiteDesignProvider } from '../state/SiteDesignContext';
import { useWhpApi } from './useWhpApi';
import { WhpConfig } from '../types';
@@ -22,8 +22,10 @@ import { WhpConfig } from '../types';
* This mocks `@craftjs/core` the same way `useWhpApi.load.test.tsx` and
* `PageContext.orphan-repair-wiring.test.tsx` do, and asserts on the exact
* string handed to the mocked `actions.deserialize` -- the orphan must be
- * reattached to ROOT and a `console.warn` must fire, exactly like the
- * page-switch path.
+ * reattached to ROOT and a `console.error` must fire, exactly like the
+ * page-switch path (I5 review: this was `console.warn`, which
+ * console-buffer.ts -- feeding the in-builder issue reporter -- does not
+ * capture).
*/
const deserializeMock = vi.fn();
vi.mock('@craftjs/core', () => ({
@@ -133,7 +135,7 @@ describe('useWhpApi load() repairs an orphaned node on the FIRST page before des
});
vi.stubGlobal('fetch', fetchMock);
- const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+ const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
const harness = render();
await act(async () => {
@@ -148,11 +150,69 @@ describe('useWhpApi load() repairs an orphaned node on the FIRST page before des
expect(parsed.stray.parent).toBe('ROOT');
// The observable signal that repair actually ran, not just that the
- // orphan happened to be absent for some unrelated reason.
- expect(warnSpy).toHaveBeenCalled();
- expect(warnSpy.mock.calls[0][0]).toContain('reattached');
+ // orphan happened to be absent for some unrelated reason. (React's own
+ // act()-environment warnings also go through console.error in this
+ // harness, so search all calls rather than assuming index 0.)
+ expect(errorSpy.mock.calls.some((call) => String(call[0]).includes('reattached'))).toBe(true);
- warnSpy.mockRestore();
+ errorSpy.mockRestore();
+ unmount();
+ });
+});
+
+/**
+ * I5 (review): `PageContext.loadState`'s deserialize failure path logs via
+ * `console.error` AND falls back to `EMPTY_CANVAS` so the user always ends
+ * up with a working (if blank) editor. `useWhpApi.load()`'s equivalent path
+ * used to be `console.warn` with NO fallback deserialize -- the initial
+ * load, which decides whether the user sees a working editor at all, both
+ * failed harder (silently leaving the Frame undeserialized) and reported
+ * quieter than every subsequent page switch. This pins the aligned
+ * behaviour.
+ */
+describe('useWhpApi load() falls back to EMPTY_CANVAS when the first page state cannot be deserialized', () => {
+ beforeEach(() => {
+ deserializeMock.mockClear();
+ });
+
+ afterEach(() => {
+ vi.unstubAllGlobals();
+ });
+
+ test('a deserialize failure on the first page logs console.error and retries with EMPTY_CANVAS', async () => {
+ const fetchMock = vi.fn().mockResolvedValue({
+ json: async () => ({
+ success: true,
+ project: {
+ design: null,
+ header_craft_state: null,
+ footer_craft_state: null,
+ pages_craft_state: [
+ { id: 'home', name: 'Home', slug: 'index', craftState: '{"ROOT":{"broken":true}}' },
+ ],
+ },
+ }),
+ });
+ vi.stubGlobal('fetch', fetchMock);
+
+ deserializeMock.mockImplementationOnce(() => {
+ throw new Error('malformed state');
+ });
+
+ const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+ const harness = render();
+
+ await act(async () => {
+ await harness.get().load();
+ });
+
+ // First call (the broken state) threw; the second call is the fallback.
+ expect(deserializeMock).toHaveBeenCalledTimes(2);
+ expect(deserializeMock.mock.calls[1][0]).toBe(EMPTY_CANVAS);
+
+ expect(errorSpy).toHaveBeenCalledWith('Failed to load page state:', expect.any(Error));
+
+ errorSpy.mockRestore();
unmount();
});
});
diff --git a/craft/src/hooks/useWhpApi.ts b/craft/src/hooks/useWhpApi.ts
index de87ba0..28f321c 100644
--- a/craft/src/hooks/useWhpApi.ts
+++ b/craft/src/hooks/useWhpApi.ts
@@ -1,7 +1,7 @@
import { useCallback } from 'react';
import { useEditor } from '@craftjs/core';
import { useEditorConfig } from '../state/EditorConfigContext';
-import { usePages } from '../state/PageContext';
+import { usePages, EMPTY_CANVAS } from '../state/PageContext';
import { useSiteDesign, SiteDesign } from '../state/SiteDesignContext';
import { exportBodyHtml } from '../utils/html-export';
import { repairOrphanNodes } from '../utils/orphan-repair';
@@ -321,14 +321,27 @@ export function useWhpApi() {
? firstPage.craftState : JSON.stringify(firstPage.craftState);
const { state, repaired } = repairOrphanNodes(rawState);
if (repaired.length > 0) {
- console.warn(
+ // I5: console-buffer.ts only patches console.error, and this
+ // reattach signal is the single most diagnostic clue for the
+ // still-unreproduced "elements drop off the canvas" report --
+ // it must reach the in-builder issue reporter's console buffer.
+ console.error(
`[site-builder] reattached ${repaired.length} unreachable node(s) to the page root:`,
repaired.join(', '),
);
}
actions.deserialize(state);
} catch (e) {
- console.warn('Failed to load page state:', e);
+ // I5: this is the initial load that decides whether the user
+ // sees a working editor at all -- align with `loadState`'s
+ // behaviour (console.error + a known-safe fallback) instead of
+ // warning quietly and leaving the Frame on whatever it last had.
+ console.error('Failed to load page state:', e);
+ try {
+ actions.deserialize(EMPTY_CANVAS);
+ } catch (_e2) {
+ // give up
+ }
}
}
diff --git a/craft/src/panels/left/PagesPanel.confirmStates.test.tsx b/craft/src/panels/left/PagesPanel.confirmStates.test.tsx
new file mode 100644
index 0000000..871eed3
--- /dev/null
+++ b/craft/src/panels/left/PagesPanel.confirmStates.test.tsx
@@ -0,0 +1,119 @@
+import { describe, test, expect } from 'vitest';
+import React from 'react';
+import { renderEditorHarness } from '../../test-utils/editorHarness';
+import { PageProvider, usePages } from '../../state/PageContext';
+import { PagesPanel } from './PagesPanel';
+
+function clickByLabel(container: HTMLElement, label: string): HTMLButtonElement {
+ const btn = container.querySelector(`[aria-label="${label}"]`) as HTMLButtonElement | null;
+ if (!btn) throw new Error(`No button found with aria-label "${label}"`);
+ return btn;
+}
+
+function clickByText(container: HTMLElement, text: string): HTMLButtonElement {
+ const btn = Array.from(container.querySelectorAll('button')).find(
+ (b) => b.textContent === text,
+ ) as HTMLButtonElement | undefined;
+ if (!btn) throw new Error(`No button found with text "${text}"`);
+ return btn;
+}
+
+/**
+ * I3 (review): `deleteConfirmId`, `resetConfirmId` and `editingId` are three
+ * independent bits of local state, but the row rendering treats them as
+ * mutually exclusive (edit -> delete -> reset -> normal, first match wins).
+ * Before this fix, none of the three setters cleared the other two, so if
+ * both `resetConfirmId` and `deleteConfirmId` were ever set to the same page
+ * id, the delete-confirm view would win (it's checked first) and cancelling
+ * it would fall through to reveal the reset-confirm view "unbidden" -- a
+ * destructive prompt the user never asked for.
+ *
+ * This is reachable any time two arm-clicks land in the same render pass
+ * (e.g. a fast double click / synthetic dual dispatch before React commits
+ * the first click's re-render) -- reproduced below by issuing both clicks
+ * inside a single `act()` batch, which is exactly what removes the re-render
+ * that would otherwise make the second button disappear before it can be
+ * clicked.
+ */
+describe('PagesPanel confirmation-state isolation (I3 review finding)', () => {
+ async function setupTwoPages() {
+ let ctx: ReturnType | null = null;
+ const Probe: React.FC = () => { ctx = usePages(); return null; };
+ const harness = renderEditorHarness();
+ harness.mountChild(
+
+
+
+ ,
+ );
+ harness.act(() => { ctx!.addPage('About', 'about'); });
+ await harness.act(async () => { await new Promise((resolve) => setTimeout(resolve, 10)); });
+ return harness;
+ }
+
+ test('arming Reset then Delete for the same page in one batch shows Delete (render precedence), and cancelling Delete does NOT reveal a leftover Reset prompt', async () => {
+ const harness = await setupTwoPages();
+
+ // Both arm-clicks land before any re-render commits.
+ harness.act(() => {
+ clickByLabel(harness.container, 'Reset About to blank').click();
+ clickByLabel(harness.container, 'Delete About').click();
+ });
+ expect(harness.container.textContent).toContain('Delete "About"?');
+ expect(harness.container.textContent).not.toContain('Clear every element from "About"?');
+
+ harness.act(() => {
+ clickByText(harness.container, 'Cancel').click();
+ });
+
+ // The fix: arming Delete clears any pending Reset confirmation for the
+ // same page, so cancelling Delete returns to the normal row, not a
+ // surprise Reset prompt.
+ expect(harness.container.textContent).not.toContain('Clear every element from "About"?');
+ expect(harness.container.textContent).not.toContain('Delete "About"?');
+
+ harness.unmount();
+ });
+
+ test('arming Delete then Reset for the same page in one batch shows Reset, and cancelling Reset does NOT reveal a leftover Delete prompt', async () => {
+ const harness = await setupTwoPages();
+
+ harness.act(() => {
+ clickByLabel(harness.container, 'Delete About').click();
+ clickByLabel(harness.container, 'Reset About to blank').click();
+ });
+ expect(harness.container.textContent).toContain('Clear every element from "About"?');
+
+ harness.act(() => {
+ clickByText(harness.container, 'Cancel').click();
+ });
+
+ expect(harness.container.textContent).not.toContain('Delete "About"?');
+ expect(harness.container.textContent).not.toContain('Clear every element from "About"?');
+
+ harness.unmount();
+ });
+
+ test('starting a Rename while a Reset confirmation is pending for the same page clears the pending Reset', async () => {
+ const harness = await setupTwoPages();
+
+ // Arm Reset, then (same batch) start editing the same row -- edit wins
+ // in render precedence, but the fix also clears resetConfirmId so
+ // cancelling the rename doesn't fall through to a stale Reset prompt.
+ harness.act(() => {
+ clickByLabel(harness.container, 'Reset About to blank').click();
+ clickByLabel(harness.container, 'Rename About').click();
+ });
+ const nameInput = harness.container.querySelector('input.control-input') as HTMLInputElement | null;
+ expect(nameInput).toBeTruthy();
+ expect(nameInput!.value).toBe('About');
+
+ harness.act(() => {
+ clickByText(harness.container, 'Cancel').click();
+ });
+
+ expect(harness.container.textContent).not.toContain('Clear every element from "About"?');
+
+ harness.unmount();
+ });
+});
diff --git a/craft/src/panels/left/PagesPanel.tsx b/craft/src/panels/left/PagesPanel.tsx
index f0ecd46..76ece38 100644
--- a/craft/src/panels/left/PagesPanel.tsx
+++ b/craft/src/panels/left/PagesPanel.tsx
@@ -77,6 +77,7 @@ export const PagesPanel: React.FC = () => {
setEditName(page.name);
setEditSlug(page.slug);
setDeleteConfirmId(null);
+ setResetConfirmId(null);
};
const autoSlug = (name: string): string => {
@@ -515,7 +516,10 @@ export const PagesPanel: React.FC = () => {
{pages.length > 1 && !isLanding && (