Fix tt/visual image drop corrupting child structure

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<TBResource> => {
   const props = await createVisualProperties(input)
 
@@ -43,7 +38,7 @@ const createTTVisualNode = async (input: TBResource): Promise<TBResource> => {
       children: [
         {
           type: 'tt/visual/image',
-          class: 'block',
+          class: 'void',
           children: [{ text: props.href }]
         },
         {
@@ -62,20 +57,32 @@ const createTTVisualNode = async (input: TBResource): Promise<TBResource> => {
 }
 
 /**
-* 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'))
     }
 

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

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 }]
-    }
-  ]
-}

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')
+  })
+})

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')
+  })
+})