Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { hashAttrs, stripHash } from '../utils'
import { hashAttrs, isEmptyMarkup, jsonToMarkup, stripHash } from '../utils'
import { MarkupNodeType } from '../model'
import { nodeDoc, nodeGif, nodeParagraph, nodeText } from '../dsl'

describe('hashAttrs', () => {
it('should return a hash of length 8', () => {
Expand Down Expand Up @@ -47,3 +49,31 @@ describe('stripHash', () => {
expect(result).toEqual(name)
})
})

describe('isEmptyMarkup with a gif node', () => {
// the enum entry must exist, or every serializer case for gif is unreachable.
it('should define MarkupNodeType.gif', () => {
expect(MarkupNodeType.gif).toBe('gif')
})

// a gif-only message must not read as empty, or the composer's send button never
// enables (ReferenceInput.svelte binds canSubmit to !isEmptyMarkup). emoji is already on
// the nonEmptyNodes allowlist for exactly this reason.
it('should not report a document containing only a gif as empty', () => {
const doc = nodeDoc(nodeParagraph(nodeGif({ 'file-id': 'blob-1', width: 320, height: 240 })))
expect(isEmptyMarkup(jsonToMarkup(doc))).toBe(false)
})

it('should still report a document with an empty paragraph as empty', () => {
expect(isEmptyMarkup(jsonToMarkup(nodeDoc(nodeParagraph())))).toBe(true)
})

it('should still report a document with text as not empty', () => {
expect(isEmptyMarkup(jsonToMarkup(nodeDoc(nodeParagraph(nodeText('hi')))))).toBe(false)
})

it('should report undefined and the empty string as empty', () => {
expect(isEmptyMarkup(undefined)).toBe(true)
expect(isEmptyMarkup('')).toBe(true)
})
})
10 changes: 10 additions & 0 deletions foundations/core/packages/text-core/src/markup/dsl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ export function nodeImage (attrs: { src: string, alt?: string, width?: number, h
return { type: MarkupNodeType.image, attrs }
}

export function nodeGif (attrs: {
'file-id'?: string
src?: string
alt?: string
width?: number
height?: number
}): MarkupNode {
return { type: MarkupNodeType.gif, attrs }
}

export function nodeReference (attrs: { id: string, label: string, objectclass: string }): MarkupNode {
return { type: MarkupNodeType.reference, attrs }
}
Expand Down
1 change: 1 addition & 0 deletions foundations/core/packages/text-core/src/markup/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export enum MarkupNodeType {
file = 'file',
reference = 'reference',
emoji = 'emoji',
gif = 'gif',
hard_break = 'hardBreak',
ordered_list = 'orderedList',
bullet_list = 'bulletList',
Expand Down
1 change: 1 addition & 0 deletions foundations/core/packages/text-core/src/markup/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ const nonEmptyNodes = [
MarkupNodeType.image,
MarkupNodeType.reference,
MarkupNodeType.emoji,
MarkupNodeType.gif,
MarkupNodeType.subLink,
MarkupNodeType.table
]
Expand Down
47 changes: 47 additions & 0 deletions foundations/core/packages/text-html/src/__tests__/html.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -579,3 +579,50 @@ describe('htmlToMarkup', () => {
})
})
})

describe('gif node', () => {
const gifDoc = (attrs: Record<string, any>): MarkupNode => ({
type: 'doc' as any,
content: [{ type: 'paragraph' as any, content: [{ type: 'gif' as any, attrs }] }]
})

// the one that matters. The image case in this serializer reads attrs.src ONLY, so a
// gif case copied from it would emit src="undefined" for a library gif and lose the blob.
it('emits a resolvable src for a gif with file-id set and src null', () => {
const html = markupToHtml(gifDoc({ 'file-id': 'blob-1', src: null, width: 320 }))
expect(html).toContain('blob-1')
expect(html).not.toContain('undefined')
})

it('emits the external src for a gif with no file-id', () => {
const html = markupToHtml(gifDoc({ 'file-id': null, src: 'https://media.example.com/x.gif?cid=1' }))
expect(html).toContain('https://media.example.com/x.gif?cid=1')
expect(html).not.toContain('undefined')
})

// img is mapped to MarkupNodeType.image unconditionally by default. Without the
// data-type discrimination a gif comes back as an image node, which the message composers
// drop because image is disabled in their kitOptions. Silent message loss.
it('parses its own html back to a gif node, not an image node', () => {
const back = htmlToMarkup(markupToHtml(gifDoc({ 'file-id': 'blob-1' })))
const found: string[] = []
const walk = (n: any): void => {
if (n?.type != null) found.push(n.type)
;(n?.content ?? []).forEach(walk)
}
walk(back)
expect(found).toContain('gif')
expect(found).not.toContain('image')
})

it('still parses a plain img without data-type as an image node', () => {
const found: string[] = []
const walk = (n: any): void => {
if (n?.type != null) found.push(n.type)
;(n?.content ?? []).forEach(walk)
}
walk(htmlToMarkup('<p><img src="https://example.com/a.png"></p>'))
expect(found).toContain('image')
expect(found).not.toContain('gif')
})
})
15 changes: 11 additions & 4 deletions foundations/core/packages/text-html/src/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ interface HtmlTagHandler {
}

interface HtmlNodeRule {
node: MarkupNodeType
// A function lets one tag map to different node types by attribute, e.g. an <img> that is a
// gif rather than an image. Without it a gif round-tripped through HTML comes back as an
// image node, which the message composers drop because image is off in their kitOptions.
node: MarkupNodeType | ((attrs: Record<string, string>) => MarkupNodeType)
getAttrs?: Record<string, AttrValue> | ((attrs: Record<string, string>) => Record<string, AttrValue> | undefined)
wrapNode?: boolean
wrapContent?: boolean
Expand Down Expand Up @@ -134,6 +137,7 @@ class HtmlParseState {

function nodeHandler ({ node, getAttrs, wrapContent, wrapNode }: HtmlNodeRule): HtmlTagHandler {
const wrapStack: boolean[] = []
const nodeStack: MarkupNodeType[] = []

return {
handleOpenTag: (state: HtmlParseState, tag: string, attributes: Record<string, string>) => {
Expand All @@ -151,7 +155,9 @@ function nodeHandler ({ node, getAttrs, wrapContent, wrapNode }: HtmlNodeRule):
}
wrapStack.push(shouldWrapNode)

state.openNode(node, attrs)
const nodeType = typeof node === 'function' ? node(attributes) : node
nodeStack.push(nodeType)
state.openNode(nodeType, attrs)

if (wrapContent === true) {
state.openNode(MarkupNodeType.paragraph)
Expand All @@ -162,7 +168,7 @@ function nodeHandler ({ node, getAttrs, wrapContent, wrapNode }: HtmlNodeRule):
state.closeNode(MarkupNodeType.paragraph)
}

state.closeNode(node)
state.closeNode(nodeStack.pop() ?? (typeof node === 'function' ? node({}) : node))
if (wrapStack.pop() === true) {
state.closeNode(MarkupNodeType.paragraph)
}
Expand Down Expand Up @@ -317,7 +323,8 @@ const nodeRules: Record<string, HtmlNodeRule> = {
node: MarkupNodeType.list_item
},
img: {
node: MarkupNodeType.image,
node: (attributes: Record<string, string>) =>
attributes['data-type'] === 'gif' ? MarkupNodeType.gif : MarkupNodeType.image,
wrapNode: true,
getAttrs: (attributes: Record<string, string>) => {
return {
Expand Down
19 changes: 19 additions & 0 deletions foundations/core/packages/text-html/src/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,25 @@ function addNodeContent (builder: NodeBuilder, node?: MarkupNode): void {
builder.closeTag('pre')
} else if (node.type === MarkupNodeType.text) {
builder.addText(node.text ?? '')
} else if (node.type === MarkupNodeType.gif) {
// Deliberately not modelled on the image case below: that one reads attrs.src only, so a
// library gif (file-id set, src null) would serialize to src="undefined" and lose the blob
// reference. file-id wins, src is the fallback for an external source.
// Null-check the raw attrs: toString(null) yields the string 'null', so comparing the
// stringified value would treat an absent file-id as present and emit src="null".
const rawFileId = attrs['file-id']
const rawSrc = attrs.src
const imgAttrs: Record<string, string | undefined> = { 'data-type': 'gif' }
if (rawFileId != null) {
imgAttrs['file-id'] = toString(rawFileId)
imgAttrs.src = toString(rawFileId)
} else if (rawSrc != null) {
imgAttrs.src = toString(rawSrc)
}
if (attrs.alt != null) imgAttrs.alt = toString(attrs.alt)
if (attrs.width != null) imgAttrs.width = toString(attrs.width)
if (attrs.height != null) imgAttrs.height = toString(attrs.height)
builder.openTag('img', imgAttrs, { selfClosing: true })
} else if (node.type === MarkupNodeType.image) {
const src = toString(attrs.src)
const alt = toString(attrs.alt)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1250,3 +1250,49 @@ describe('normalizeMarkdown', () => {
expect(normalizeMarkdown(input)).toBe(expected)
})
})

describe('gif node', () => {
const gifDoc = (attrs: Record<string, any>): MarkupNode => ({
type: MarkupNodeType.doc,
content: [{ type: MarkupNodeType.paragraph, content: [{ type: MarkupNodeType.gif, attrs }] }]
})

// a missing serializer case is NOT silent here. serializer.ts render() throws for an
// unregistered type, which fails the WHOLE message, not just the GIF.
it('serializes without throwing', () => {
expect(() => markupToMarkdown(gifDoc({ 'file-id': 'blob-1' }), options)).not.toThrow()
})

// serialize direction on its own, so a failure localises.
it('serializes a library gif carrying its file-id', () => {
const md = markupToMarkdown(gifDoc({ 'file-id': 'blob-1', width: 320, height: 240 }), options)
expect(md).toContain('blob-1')
})

// parse direction on its own. The emoji node fails exactly here: it has no parser
// token at all, so markdown never reconstructs it and the content is lost for good.
it('round-trips a library gif back to a gif node, not an image node', () => {
const md = markupToMarkdown(gifDoc({ 'file-id': 'blob-1', width: 320, height: 240 }), options)
const back = markdownToMarkup(md, options)
const node = (back.content?.[0] as MarkupNode)?.content?.[0]
expect(node?.type).toBe(MarkupNodeType.gif)
expect(node?.attrs?.['file-id']).toBe('blob-1')
})

// an external src must survive unmodified, query params included. This is what lets a
// third-party source reuse the node later without a schema change.
it('round-trips an external src without rewriting the url', () => {
const src = 'https://media.example.com/x.gif?cid=abc&ct=g'
const back = markdownToMarkup(markupToMarkdown(gifDoc({ src }), options), options)
const node = (back.content?.[0] as MarkupNode)?.content?.[0]
expect(node?.type).toBe(MarkupNodeType.gif)
expect(node?.attrs?.src).toBe(src)
})

// idempotence, so an edit-and-resave cycle cannot corrupt via double escaping.
it('is stable across two serializations', () => {
const once = markupToMarkdown(gifDoc({ 'file-id': 'blob-1' }), options)
const twice = markupToMarkdown(markdownToMarkup(once, options), options)
expect(normalizeMarkdown(twice)).toEqual(normalizeMarkdown(once))
})
})
15 changes: 15 additions & 0 deletions foundations/core/packages/text-markdown/src/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,21 @@ export const storeNodes: Record<string, NodeProcessor> = {
}
}
},
gif: (state, node) => {
// Emitted as a tagged <img> rather than markdown image syntax, because markdown's ![](...)
// carries no way to distinguish a gif from an image and the parser would hand it back as an
// image node. 'file-id' and 'src' are independent; file-id wins.
const attrs = nodeAttrs(node)
state.write(
'<img data-type="gif"' +
(attrs['file-id'] != null ? ` file-id="${state.esc(`${attrs['file-id']}`)}"` : '') +
(attrs.src != null ? ` src="${state.esc(`${attrs.src}`)}"` : '') +
(attrs.width != null ? ` width="${state.esc(`${attrs.width}`)}"` : '') +
(attrs.height != null ? ` height="${state.esc(`${attrs.height}`)}"` : '') +
(attrs.alt != null ? ` alt="${state.esc(`${attrs.alt}`)}"` : '') +
'>'
)
},
reference: (state, node) => {
const attrs = nodeAttrs(node)
let url = state.refUrl
Expand Down
2 changes: 2 additions & 0 deletions foundations/core/packages/text/src/kits/server-kit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { NoteBaseExtension } from '../marks/noteBase'
import { QMSInlineCommentMark } from '../marks/qmsInlineCommentMark'

import { EmojiNode } from '../nodes/emoji'
import { GifNode } from '../nodes/gif'
import { FileNode } from '../nodes/file'
import { ImageNode } from '../nodes/image'
import { ReferenceNode } from '../nodes/reference'
Expand Down Expand Up @@ -47,6 +48,7 @@ export const ServerKitFactory = (e: ExtensionFactory) =>
image: e(ImageNode),
embed: e(EmbedNode),
emoji: e(EmojiNode),
gif: e(GifNode),

inlineNote: e(NoteBaseExtension), // Semi-deprecated, should be removed in the future
qmsInlineCommentMark: e(QMSInlineCommentMark) // Semi-deprecated, should be removed in the future
Expand Down
20 changes: 20 additions & 0 deletions foundations/core/packages/text/src/markup/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,3 +441,23 @@ describe('jsonToText', () => {
expect(() => jsonToText(node)).toThrow('Empty text nodes are not allowed')
})
})

describe('gif node in the server kit', () => {
// without this registration the collaborator strips the node server-side, so a GIF
// survives locally and disappears once the message syncs. The emoji node is registered the
// same way for the same reason.
it('is present in the server kit schema', () => {
const schema = getSchema(extensions)
expect(schema.nodes.gif).toBeDefined()
})

// a gif must be selectable and must not be an atom, unlike emoji. This is the one of
// the four reasons not to reuse insertEmoji that no serializer test covers.
it('is selectable and not an atom, unlike emoji', () => {
const schema = getSchema(extensions)
expect(schema.nodes.gif.spec.atom).not.toBe(true)
expect(schema.nodes.gif.spec.selectable).not.toBe(false)
expect(schema.nodes.emoji.spec.atom).toBe(true)
expect(schema.nodes.emoji.spec.selectable).toBe(false)
})
})
Loading
Loading