From 02b84892ea8b7a582d70526fc5073dd4abc8a17c Mon Sep 17 00:00:00 2001 From: Danne Lundqvist Date: Tue, 2 Jun 2026 11:22:24 +0200 Subject: [PATCH 1/2] Fix tt/visual image drop corrupting child structure --- lib/TTVisual/lib/consume.ts | 16 +++- lib/TTVisual/lib/normalizeTTVisual.ts | 19 ++++- tests/TTVisual/consume.test.ts | 66 +++++++++++++++++ tests/TTVisual/normalizeTTVisual.test.ts | 93 ++++++++++++++++++++++++ 4 files changed, 190 insertions(+), 4 deletions(-) create mode 100644 tests/TTVisual/consume.test.ts create mode 100644 tests/TTVisual/normalizeTTVisual.test.ts diff --git a/lib/TTVisual/lib/consume.ts b/lib/TTVisual/lib/consume.ts index 34e2c10..77a3f97 100644 --- a/lib/TTVisual/lib/consume.ts +++ b/lib/TTVisual/lib/consume.ts @@ -43,7 +43,7 @@ const createTTVisualNode = async (input: TBResource): Promise => { children: [ { type: 'tt/visual/image', - class: 'block', + class: 'void', children: [{ text: props.href }] }, { @@ -62,20 +62,32 @@ const createTTVisualNode = async (input: TBResource): Promise => { } /** -* Get image resolution without downloading the image +* Get image resolution without downloading the image. +* +* Rejects after 15s if neither onload nor onerror fires (e.g. CORS preflight +* stall, DNS hang). Without this, `consume()` could hang indefinitely and +* leave a phantom loader in the editor. +* * @param {string} url * @returns {Promise<{ width: number, height: number } | null>} * @throws {Error} */ +const IMAGE_RESOLUTION_TIMEOUT_MS = 15_000 + const getImageResolution = async (url: string): Promise<{ width: number, height: number } | null> => { return await new Promise((resolve, reject) => { const img = new Image() + const timer = setTimeout(() => { + reject(new Error(`Timed out loading image after ${IMAGE_RESOLUTION_TIMEOUT_MS}ms`)) + }, IMAGE_RESOLUTION_TIMEOUT_MS) img.onload = () => { + clearTimeout(timer) resolve({ width: img.width, height: img.height }) } img.onerror = () => { + clearTimeout(timer) reject(new Error('Failed to load image')) } diff --git a/lib/TTVisual/lib/normalizeTTVisual.ts b/lib/TTVisual/lib/normalizeTTVisual.ts index 16aef22..86f92b2 100644 --- a/lib/TTVisual/lib/normalizeTTVisual.ts +++ b/lib/TTVisual/lib/normalizeTTVisual.ts @@ -1,17 +1,32 @@ import { type Editor, + Element, type NodeEntry, Node, Transforms } from 'slate' import { TextbitElement } from '@ttab/textbit' +// Types this normalizer is allowed to leave alone (the schema-defined +// children of tt/visual). Anything else that ends up nested at this level +// — e.g. an unrelated block dropped/pasted inside — is unwrapped to keep +// the structure flat. The earlier, looser rule unwrapped *every* block +// child, which destroyed valid schema children when their class was wrong. +const SCHEMA_CHILD_TYPES = new Set([ + 'tt/visual/image', + 'tt/visual/text', + 'tt/visual/byline' +]) + export const normalizeTTVisual = (editor: Editor, nodeEntry: NodeEntry): boolean | undefined => { const [, path] = nodeEntry for (const [child, childPath] of Node.children(editor, path)) { - if (TextbitElement.isBlock(child)) { - // Unwrap block node children (move text element children upwards in tree) + if ( + Element.isElement(child) + && TextbitElement.isBlock(child) + && !SCHEMA_CHILD_TYPES.has(child.type) + ) { Transforms.unwrapNodes(editor, { at: childPath, split: true diff --git a/tests/TTVisual/consume.test.ts b/tests/TTVisual/consume.test.ts new file mode 100644 index 0000000..cc6bb4b --- /dev/null +++ b/tests/TTVisual/consume.test.ts @@ -0,0 +1,66 @@ +import { describe, it, expect } from 'vitest' +import { consume } from '../../lib/TTVisual/lib/consume' +import type { TBResource } from '@ttab/textbit' +import type { Editor } from 'slate' + +// A JSON input lets us test consume() end-to-end without going through the +// network-dependent `new Image()` image-resolution path. +const jsonInput: TBResource = { + data: JSON.stringify({ + type: 'tt/picture', + href: 'https://tt.se/media/image/sdlABCDEF123_WatermarkPreview.jpg', + uri: 'http://tt.se/media/image/sdlABCDEF123', + byline: 'Photographer', + text: 'Caption', + width: 800, + height: 600 + }), + type: 'tt/visual', + source: '' +} + +describe('TTVisual consume', () => { + it('throws if input is an array', async () => { + await expect(consume({ input: [jsonInput], editor: {} as Editor })).rejects.toThrow( + /expected string for consumation, not a list/ + ) + }) + + it('throws if input.data is not a string', async () => { + await expect(consume({ input: { ...jsonInput, data: 123 } as unknown as TBResource, editor: {} as Editor })).rejects.toThrow( + /expected string for consumation/ + ) + }) + + it('returns a valid tt/visual node', async () => { + const result = await consume({ input: jsonInput, editor: {} as Editor }) + expect(result?.data).toBeDefined() + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const data = result!.data as any + expect(data.class).toBe('block') + expect(data.type).toBe('tt/visual') + expect(typeof data.id).toBe('string') + expect(data.properties.href).toBe('https://tt.se/media/image/sdlABCDEF123_WatermarkPreview.jpg') + expect(data.properties.text).toBe('Caption') + expect(data.properties.byline).toBe('Photographer') + }) + + it('produces the image child with class: void (must match plugin declaration)', async () => { + // Regression test for the bug where consume() emitted the image child as + // class: 'block' while the plugin registered it as class: 'void'. The + // mismatch caused normalizeTTVisual to unwrap the image element on every + // drop, corrupting the structure under the visible href-driven render. + const result = await consume({ input: jsonInput, editor: {} as Editor }) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const data = result!.data as any + + expect(data.children).toHaveLength(3) + expect(data.children[0].type).toBe('tt/visual/image') + expect(data.children[0].class).toBe('void') + expect(data.children[1].type).toBe('tt/visual/text') + expect(data.children[1].class).toBe('text') + expect(data.children[2].type).toBe('tt/visual/byline') + expect(data.children[2].class).toBe('text') + }) +}) diff --git a/tests/TTVisual/normalizeTTVisual.test.ts b/tests/TTVisual/normalizeTTVisual.test.ts new file mode 100644 index 0000000..05ddd6c --- /dev/null +++ b/tests/TTVisual/normalizeTTVisual.test.ts @@ -0,0 +1,93 @@ +import { describe, it, expect } from 'vitest' +import { createEditor, type Descendant, type Editor } from 'slate' +import { normalizeTTVisual } from '../../lib/TTVisual/lib/normalizeTTVisual' + +function makeEditor(value: Descendant[]): Editor { + const editor = createEditor() + editor.children = value + return editor +} + +const canonicalVisual: Descendant = { + id: 'visual-1', + class: 'block', + type: 'tt/visual', + properties: { href: 'http://tt.se/media/image/sdlX_WatermarkPreview.jpg' }, + children: [ + { + id: 'visual-1-image', + type: 'tt/visual/image', + class: 'void', + children: [{ text: 'http://tt.se/media/image/sdlX_WatermarkPreview.jpg' }] + }, + { + id: 'visual-1-text', + type: 'tt/visual/text', + class: 'text', + properties: {}, + children: [{ text: 'Caption' }] + }, + { + id: 'visual-1-byline', + type: 'tt/visual/byline', + class: 'text', + properties: {}, + children: [{ text: 'Photographer' }] + } + ] +} + +describe('normalizeTTVisual', () => { + it('is a no-op on a canonical tt/visual node', () => { + const editor = makeEditor([structuredClone(canonicalVisual)]) + const result = normalizeTTVisual(editor, [editor.children[0], [0]]) + expect(result).toBeUndefined() + // children untouched + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const node = editor.children[0] as any + expect(node.children).toHaveLength(3) + expect(node.children[0].type).toBe('tt/visual/image') + expect(node.children[0].class).toBe('void') + }) + + it('unwraps a foreign block child nested inside tt/visual', () => { + const visual = structuredClone(canonicalVisual) as Descendant & { children: Descendant[] } + // Insert an unexpected block-class child (e.g. dropped/pasted) + visual.children.push({ + id: 'foreign-block', + type: 'core/image', + class: 'block', + children: [{ text: 'stray' }] + }) + + const editor = makeEditor([visual]) + const result = normalizeTTVisual(editor, [editor.children[0], [0]]) + expect(result).toBe(true) + + // The foreign block element is unwrapped: its element wrapper is removed + // and its text leaf is hoisted into the parent's children array. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const node = editor.children[0] as any + const stillHasForeign = node.children.some( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (c: any) => c.type === 'core/image' + ) + expect(stillHasForeign).toBe(false) + }) + + it('does not unwrap the tt/visual/image schema child even if class is mislabeled', () => { + // Defensive: protect against legacy data where the image child might + // have been persisted with class: 'block' (the previous bug). The + // schema-type-aware rule should leave it alone rather than destroy it. + const visual = structuredClone(canonicalVisual) as Descendant & { children: Descendant[] } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(visual.children[0] as any).class = 'block' + + const editor = makeEditor([visual]) + const result = normalizeTTVisual(editor, [editor.children[0], [0]]) + expect(result).toBeUndefined() + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const node = editor.children[0] as any + expect(node.children[0].type).toBe('tt/visual/image') + }) +}) From 35f8ede0fe6e74fc88951ff49671c5880cbe1905 Mon Sep 17 00:00:00 2001 From: Danne Lundqvist Date: Wed, 3 Jun 2026 09:33:33 +0200 Subject: [PATCH 2/2] Removed unused types --- lib/Image/types.ts | 24 ------------------------ lib/TTVisual/lib/consume.ts | 5 ----- lib/TTVisual/types.ts | 24 ------------------------ 3 files changed, 53 deletions(-) delete mode 100644 lib/Image/types.ts diff --git a/lib/Image/types.ts b/lib/Image/types.ts deleted file mode 100644 index 7ed8616..0000000 --- a/lib/Image/types.ts +++ /dev/null @@ -1,24 +0,0 @@ -export interface TTVisualInterface { - id: string - class: 'block' - type: 'core/image' - properties: { - type: string - src: string - title: string - size: number - width: number - height: number - } - children: [ - { - type: 'core/image/image' - children: [{ text: '' }] - }, - { - type: 'core/image/text' - class: 'text' - children: [{ text: string }] - } - ] -} diff --git a/lib/TTVisual/lib/consume.ts b/lib/TTVisual/lib/consume.ts index 77a3f97..bf85364 100644 --- a/lib/TTVisual/lib/consume.ts +++ b/lib/TTVisual/lib/consume.ts @@ -15,11 +15,6 @@ export const consume: TBConsumeFunction = async ({ input }) => { return createTTVisualNode(input) } -/** -* Create a TTVisual node -* @param {VisualPropertiesInterface} props -* @returns {TTVisualInterface} -*/ const createTTVisualNode = async (input: TBResource): Promise => { const props = await createVisualProperties(input) diff --git a/lib/TTVisual/types.ts b/lib/TTVisual/types.ts index abeb86f..3cc8bd4 100644 --- a/lib/TTVisual/types.ts +++ b/lib/TTVisual/types.ts @@ -9,27 +9,3 @@ export interface VisualPropertiesInterface { width: number type: 'tt/picture' } - -export interface TTVisualInterface { - id: string - class: 'block' - type: 'tt/visual' - properties: VisualPropertiesInterface - children: [ - { - type: 'tt/visual/image' - class?: 'block' - children: [{ text: string }] - }, - { - type: 'tt/visual/text' - class: 'text' - children: [{ text: string }] - }, - { - type: 'tt/visual/byline' - class: 'text' - children: [{ text: string }] - } - ] -}