From 407dff7c0bbc30fc8e3fc5a22ef5d52d817a409a Mon Sep 17 00:00:00 2001 From: Renato Atilio Date: Wed, 5 Aug 2026 15:13:32 -0300 Subject: [PATCH 1/5] FEATURE: Support all bbcode tags in the rich editor Opening a post that used any of this plugin's tags ([color], [size], [left], [list=a], etc.) in the rich text editor failed with "The rich text editor doesn't support all features used in this post" and forced the markdown editor. Add a rich editor extension covering every tag the plugin supports: inline styling as marks, alignment, [indent], [ot] and [edit] as block nodes, and typed [list=X] lists. A tag value is only accepted when it matches the charsets the cook sanitizer allows, so the editor can't show styling the rendered post drops, and a value can't carry extra style declarations into the editor's DOM. Content the editor can't represent exactly is declined rather than rewritten, leaving the post to the markdown editor with its source intact: values outside those charsets, and a same-type tag nested in one with a differing value, since a mark set holds one mark per type and nested [size] percentages compound when cooked. Cooking changes with it, without changing what a post renders as. Cooked sepquotes carry a data-tag, so [ot] and [edit] are told apart structurally instead of by their localized label, which no longer works once a post is read under another locale. Typed lists get their own token type and every item's content is wrapped in hidden paragraph tokens, which render as nothing but let prosemirror-markdown's own list specs parse them. --- .../api-initializers/bbcode-rich-editor.js | 6 + .../discourse/lib/rich-editor-extension.js | 500 ++++++++++++++++++ .../lib/discourse-markdown/bbcode.js | 28 +- assets/stylesheets/bbcode.scss | 3 +- spec/pretty_text_spec.rb | 4 +- .../integration/bbcode-round-trip-test.js | 96 ++++ .../integration/rich-editor-extension-test.js | 350 ++++++++++++ test/javascripts/lib/bbcode-cooked-test.js | 21 + 8 files changed, 998 insertions(+), 10 deletions(-) create mode 100644 assets/javascripts/discourse/api-initializers/bbcode-rich-editor.js create mode 100644 assets/javascripts/discourse/lib/rich-editor-extension.js create mode 100644 test/javascripts/integration/bbcode-round-trip-test.js create mode 100644 test/javascripts/integration/rich-editor-extension-test.js create mode 100644 test/javascripts/lib/bbcode-cooked-test.js diff --git a/assets/javascripts/discourse/api-initializers/bbcode-rich-editor.js b/assets/javascripts/discourse/api-initializers/bbcode-rich-editor.js new file mode 100644 index 0000000..dbda0b3 --- /dev/null +++ b/assets/javascripts/discourse/api-initializers/bbcode-rich-editor.js @@ -0,0 +1,6 @@ +import { apiInitializer } from "discourse/lib/api"; +import richEditorExtension from "../lib/rich-editor-extension"; + +export default apiInitializer((api) => { + api.registerRichEditorExtension(richEditorExtension); +}); diff --git a/assets/javascripts/discourse/lib/rich-editor-extension.js b/assets/javascripts/discourse/lib/rich-editor-extension.js new file mode 100644 index 0000000..4c23d66 --- /dev/null +++ b/assets/javascripts/discourse/lib/rich-editor-extension.js @@ -0,0 +1,500 @@ +import { serializeBBCodeAttr } from "discourse/lib/text"; +import { i18n } from "discourse-i18n"; + +const ALIGNMENTS = ["left", "right", "center"]; + +// the cook sanitizer's allowlist: a looser value would style the editor but +// be stripped from the rendered post +const SIZE_VALUE = /^\d{1,3}%$/; +const FONT_VALUE = /^[a-zA-Z0-9\s-]+$/; +const COLOR_VALUE = /^#?[a-zA-Z0-9]+$/; + +// from a browser these mean "no color", unlike an authored [color=transparent] +const NON_COLORS = [ + "transparent", + "currentcolor", + "inherit", + "initial", + "unset", + "revert", +]; + +function normalizeColor(value) { + const rgb = value.match( + /^rgba?\((\d+)[,\s]+(\d+)[,\s]+(\d+)(?:[,\s/]+([\d.]+))?\)/ + ); + if (rgb) { + // bbcode can't express partial transparency + if (rgb[4] !== undefined && parseFloat(rgb[4]) < 1) { + return null; + } + return ( + "#" + + rgb + .slice(1, 4) + .map((channel) => (+channel).toString(16).padStart(2, "0")) + .join("") + ); + } + if (NON_COLORS.includes(value.toLowerCase())) { + return null; + } + return COLOR_VALUE.test(value) ? value : null; +} + +function unquoteFont(value) { + const font = value.match(/^(['"]?)(.+)\1$/)?.[2]; + return font && FONT_VALUE.test(font) ? font : null; +} + +// a Map, so a property named like an Object member can't hit the prototype +const SPAN_MARKS = new Map([ + [ + "font-size", + (value, marks) => + value === "x-small" + ? marks.bbcode_small.create() + : SIZE_VALUE.test(value) && + marks.bbcode_size.create({ size: parseInt(value, 10) }), + ], + [ + "font-family", + (value, marks) => { + const font = unquoteFont(value); + return font && marks.bbcode_font.create({ font }); + }, + ], + [ + "color", + (value, marks) => + COLOR_VALUE.test(value) && marks.bbcode_color.create({ color: value }), + ], + [ + "background-color", + (value, marks) => + COLOR_VALUE.test(value) && marks.bbcode_bgcolor.create({ color: value }), + ], +]); + +function inlineMarkFor(token, schema) { + if (token.tag === "span") { + const [, property, value] = + token.attrGet("style")?.match(/^([\w-]+):(.+)$/) ?? []; + + return SPAN_MARKS.get(property)?.(value, schema.marks) || null; + } + + if (token.tag === "a") { + const name = serializableAttr(token.attrGet("name")); + if (name) { + return schema.marks.bbcode_aname.create({ name }); + } + + const href = token.attrGet("href"); + if (href?.startsWith("#")) { + const anchor = serializableAttr(href.slice(1)); + return anchor && schema.marks.bbcode_jumpto.create({ anchor }); + } + } + + return null; +} + +// a bbcode tag is a single line, so no quoting can hold a newline +function serializableAttr(value) { + return value && !value.includes("\n") ? value : null; +} + +// a mark set holds one per type: an identical nesting adds nothing, a differing +// one can't be represented, so it's declined and the post stays in markdown +function openInlineMark(state, mark) { + const open = (state.bbcodeInlineMarks ??= []); + const enclosing = open.find((entry) => entry?.type === mark.type); + + if (enclosing) { + if (!enclosing.eq(mark)) { + return false; + } + + // balances the matching close + open.push(null); + return true; + } + + state.openMark(mark); + open.push(mark); + return true; +} + +function closeInlineMark(state) { + const mark = state.bbcodeInlineMarks.pop(); + if (mark) { + state.closeMark(mark); + } +} + +function wrapInTag(state, node, tag) { + state.write(`[${tag}]\n`); + state.renderContent(node); + state.flushClose(1); + state.write(`[/${tag}]`); + state.closeBlock(node); +} + +function colorMark(property) { + return { + attrs: { color: {} }, + parseDOM: [ + { + style: property, + getAttrs: (value) => { + const color = normalizeColor(value); + return color ? { color } : false; + }, + }, + ], + toDOM: (mark) => ["span", { style: `${property}:${mark.attrs.color}` }, 0], + }; +} + +// String() so a numeric value isn't mistaken for an absent one +function bbcodeTag(tag, value) { + return `[${serializeBBCodeAttr(String(value), tag).trim()}]`; +} + +function attrSerializer(tag, attr) { + return { + open: (state, mark) => bbcodeTag(tag, mark.attrs[attr]), + close: `[/${tag}]`, + mixable: true, + expelEnclosingWhitespace: true, + }; +} + +/** @type {RichEditorExtension} */ +const extension = { + markSpec: { + bbcode_size: { + attrs: { size: {} }, + parseDOM: [ + { + style: "font-size", + getAttrs: (value) => + SIZE_VALUE.test(value) ? { size: parseInt(value, 10) } : false, + }, + ], + toDOM: (mark) => ["span", { style: `font-size:${mark.attrs.size}%` }, 0], + }, + bbcode_font: { + attrs: { font: {} }, + parseDOM: [ + { + style: "font-family", + getAttrs: (value) => { + const font = unquoteFont(value); + return font ? { font } : false; + }, + }, + ], + toDOM: (mark) => [ + "span", + { style: `font-family:'${mark.attrs.font}'` }, + 0, + ], + }, + bbcode_color: colorMark("color"), + bbcode_bgcolor: colorMark("background-color"), + bbcode_highlight: { + parseDOM: [{ tag: "span.highlight" }], + toDOM: () => ["span", { class: "highlight" }, 0], + }, + bbcode_small: { + parseDOM: [{ style: "font-size=x-small" }], + toDOM: () => ["span", { style: "font-size:x-small" }, 0], + }, + bbcode_aname: { + attrs: { name: {} }, + parseDOM: [ + { + tag: "a[name]", + getAttrs: (dom) => { + const name = serializableAttr(dom.getAttribute("name")); + return name ? { name } : false; + }, + }, + ], + toDOM: (mark) => ["a", { name: mark.attrs.name }, 0], + }, + bbcode_jumpto: { + attrs: { anchor: {} }, + parseDOM: [ + { + tag: "a[href^='#']", + priority: 60, + getAttrs: (dom) => { + const anchor = serializableAttr(dom.getAttribute("href").slice(1)); + return anchor ? { anchor } : false; + }, + }, + ], + toDOM: (mark) => ["a", { href: `#${mark.attrs.anchor}` }, 0], + }, + }, + + nodeSpec: { + bbcode_align: { + attrs: { align: {} }, + group: "block", + content: "block+", + defining: true, + createGapCursor: true, + parseDOM: [ + { + tag: "div[style*=text-align]", + getAttrs: (dom) => + ALIGNMENTS.includes(dom.style.textAlign) + ? { align: dom.style.textAlign } + : false, + }, + ], + toDOM: (node) => ["div", { style: `text-align:${node.attrs.align}` }, 0], + }, + bbcode_indent: { + group: "block", + content: "block+", + defining: true, + createGapCursor: true, + parseDOM: [{ tag: "blockquote.indent", priority: 60 }], + toDOM: () => ["blockquote", { class: "indent" }, 0], + }, + bbcode_sepquote: { + attrs: { tag: { default: "ot" } }, + group: "block", + content: "block+", + defining: true, + createGapCursor: true, + parseDOM: [ + { + tag: "div.sepquote", + getAttrs: (dom) => { + if (["edit", "ot"].includes(dom.dataset.tag)) { + return { tag: dom.dataset.tag }; + } + + const label = dom + .querySelector("span.smallfont") + ?.textContent.trim(); + if (label === i18n("bbcode.edit")) { + return { tag: "edit" }; + } + if (label === i18n("bbcode.ot")) { + return { tag: "ot" }; + } + + return false; + }, + }, + // cooked sepquotes decorate the content with a localized label + { tag: "div.sepquote > span.smallfont", ignore: true }, + ], + toDOM: (node) => [ + "div", + { class: "sepquote", "data-tag": node.attrs.tag }, + 0, + ], + }, + // an ordered list with an explicit list-style type, e.g. [list=a] + bbcode_list: { + attrs: { type: {}, tight: { default: true } }, + group: "block", + content: "list_item+", + parseDOM: [ + { + tag: "ol[type]", + priority: 60, + getAttrs: (dom) => { + const type = serializableAttr(dom.getAttribute("type")); + return type ? { type } : false; + }, + }, + ], + toDOM: (node) => ["ol", { type: node.attrs.type }, 0], + }, + }, + + parse: { + bbcode_open(state, token) { + const mark = inlineMarkFor(token, state.schema); + return !!mark && openInlineMark(state, mark); + }, + + bbcode_close(state, token) { + if ( + (token.tag === "span" || token.tag === "a") && + state.bbcodeInlineMarks?.length + ) { + closeInlineMark(state); + return true; + } + }, + + bbcode_highlight_open(state) { + return openInlineMark( + state, + state.schema.marks.bbcode_highlight.create() + ); + }, + + bbcode_highlight_close(state) { + if (state.bbcodeInlineMarks?.length) { + closeInlineMark(state); + return true; + } + }, + + // shared with any wrapping block bbcode tag, so track which opens were ours + wrap_bbcode(state, token) { + if (token.nesting === 1) { + let opened = false; + + if (token.tag === "div") { + const align = token + .attrGet("style") + ?.match(/^text-align:(\w+)$/)?.[1]; + + if (ALIGNMENTS.includes(align)) { + state.openNode(state.schema.nodes.bbcode_align, { align }); + opened = true; + } + } else if ( + token.tag === "blockquote" && + token.attrGet("class") === "indent" + ) { + state.openNode(state.schema.nodes.bbcode_indent); + opened = true; + } + + (state.bbcodeWraps ??= []).push(opened); + if (opened) { + return true; + } + } else if (token.nesting === -1 && state.bbcodeWraps?.length) { + if (state.bbcodeWraps.pop()) { + state.closeNode(); + return true; + } + } + }, + + sepquote_open(state, token, tokens, i) { + // the localized label is regenerated on cook; expected token order: + // sepquote_open span_open text span_close soft_break² + const label = tokens[i + 2]; + if (label?.type === "text") { + label.content = ""; + } + + state.openNode(state.schema.nodes.bbcode_sepquote, { + tag: token.attrGet("data-tag") === "edit" ? "edit" : "ot", + }); + return true; + }, + + sepquote_close(state) { + if (state.top().type.name === "bbcode_sepquote") { + state.closeNode(); + return true; + } + }, + + span_open(state, token) { + if ( + token.attrGet("class") === "smallfont" && + state.top().type.name === "bbcode_sepquote" + ) { + return true; + } + }, + + span_close(state) { + if (state.top().type.name === "bbcode_sepquote") { + return true; + } + }, + + soft_break(state) { + if (state.top().type.name === "bbcode_sepquote") { + return true; + } + }, + + bbcode_list: { + block: "bbcode_list", + getAttrs: (token) => ({ type: token.attrGet("type") }), + }, + }, + + plugins: ({ pmState: { Plugin } }) => + new Plugin({ + props: { + // an ignored
still opens a textblock while parsing, so the + // cooked label's separators have to go before that + transformPastedHTML(html) { + if (!html.includes("sepquote")) { + return html; + } + + const doc = new DOMParser().parseFromString(html, "text/html"); + doc + .querySelectorAll("div.sepquote > br") + .forEach((br) => br.remove()); + return doc.body.innerHTML; + }, + }, + }), + + serializeMark: { + bbcode_size: attrSerializer("size", "size"), + bbcode_font: attrSerializer("font", "font"), + bbcode_color: attrSerializer("color", "color"), + bbcode_bgcolor: attrSerializer("bgcolor", "color"), + bbcode_aname: attrSerializer("aname", "name"), + bbcode_jumpto: attrSerializer("jumpto", "anchor"), + bbcode_highlight: { + open: "[highlight]", + close: "[/highlight]", + mixable: true, + expelEnclosingWhitespace: true, + }, + bbcode_small: { + open: "[small]", + close: "[/small]", + mixable: true, + expelEnclosingWhitespace: true, + }, + }, + + serializeNode: { + bbcode_align(state, node) { + wrapInTag(state, node, node.attrs.align); + }, + + bbcode_indent(state, node) { + wrapInTag(state, node, "indent"); + }, + + bbcode_sepquote(state, node) { + wrapInTag(state, node, node.attrs.tag); + }, + + bbcode_list(state, node) { + state.write(`${bbcodeTag("list", node.attrs.type)}\n`); + state.renderList(node, "", () => "[*]"); + state.flushClose(1); + state.write("[/list]"); + state.closeBlock(node); + }, + }, +}; + +export default extension; diff --git a/assets/javascripts/lib/discourse-markdown/bbcode.js b/assets/javascripts/lib/discourse-markdown/bbcode.js index f8c8492..23b304e 100644 --- a/assets/javascripts/lib/discourse-markdown/bbcode.js +++ b/assets/javascripts/lib/discourse-markdown/bbcode.js @@ -100,7 +100,10 @@ function setupMarkdownIt(md) { tag, before: function (state) { let token = state.push("sepquote_open", "div", 1); - token.attrs = [["class", "sepquote"]]; + token.attrs = [ + ["class", "sepquote"], + ["data-tag", tag], + ]; token = state.push("span_open", "span", 1); token.block = false; @@ -125,13 +128,14 @@ function setupMarkdownIt(md) { tag, replace: function (state, tagInfo, content) { let ol = tag === "ol" || (tag === "list" && tagInfo.attrs._default); + let type = ol ? tagInfo.attrs._default : null; let token; - if (ol) { - token = state.push("ordered_list_open", "ol", 1); - if (tagInfo.attrs._default) { - token.attrs = [["type", tagInfo.attrs._default]]; - } + if (type) { + token = state.push("bbcode_list_open", "ol", 1); + token.attrs = [["type", type]]; + } else if (ol) { + state.push("ordered_list_open", "ol", 1); } else { state.push("bullet_list_open", "ul", 1); } @@ -167,17 +171,27 @@ function setupMarkdownIt(md) { list.forEach((li) => { if (li !== null) { state.push("list_item_open", "li", 1); + + // hidden, as markdown-it wraps tight list items: renders as + // nothing, but makes the item's content a paragraph like anywhere + // else, instead of a bare inline token + state.push("paragraph_open", "p", 1).hidden = true; + // a bit lazy, we could use a block parser here // but it means a lot of fussing with line marks token = state.push("inline", "", 0); token.content = li; token.children = []; + state.push("paragraph_close", "p", -1).hidden = true; + state.push("list_item_close", "li", -1); } }); - if (ol) { + if (type) { + state.push("bbcode_list_close", "ol", -1); + } else if (ol) { state.push("ordered_list_close", "ol", -1); } else { state.push("bullet_list_close", "ul", -1); diff --git a/assets/stylesheets/bbcode.scss b/assets/stylesheets/bbcode.scss index 7fcb37f..aad9ff7 100644 --- a/assets/stylesheets/bbcode.scss +++ b/assets/stylesheets/bbcode.scss @@ -1,5 +1,6 @@ .d-editor-preview, -.cooked { +.cooked, +.ProseMirror { span.highlight { background-color: var(--bbcode-highlight); padding: 2px; diff --git a/spec/pretty_text_spec.rb b/spec/pretty_text_spec.rb index 1cefe16..1c4b16a 100644 --- a/spec/pretty_text_spec.rb +++ b/spec/pretty_text_spec.rb @@ -110,13 +110,13 @@ cooked = PrettyText.cook(markdown) html = <<~HTML -
+
Off Topic:

test

-
+
Edit:

diff --git a/test/javascripts/integration/bbcode-round-trip-test.js b/test/javascripts/integration/bbcode-round-trip-test.js new file mode 100644 index 0000000..58b19aa --- /dev/null +++ b/test/javascripts/integration/bbcode-round-trip-test.js @@ -0,0 +1,96 @@ +import { module, test } from "qunit"; +import { + registerRichEditorExtension, + resetRichEditorExtensions, +} from "discourse/lib/composer/rich-editor-extensions"; +import { cook } from "discourse/lib/text"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; +import { setupRichEditor } from "discourse/tests/helpers/rich-editor-helper"; +import richEditorExtension from "discourse/plugins/discourse-bbcode/discourse/lib/rich-editor-extension"; + +// the source may be normalized, but it has to keep cooking to the same post. +// the second entry is the equivalent source for tags the editor rewrites to +// markdown, which cook renders with the same styling under other tags. +const CASES = [ + ["[color=red]red[/color] text"], + ["[color=#ff0000]hex[/color] text"], + ["[bgcolor=yellow]marked[/bgcolor] text"], + ["[size=150]large[/size] text"], + ["[font=courier]mono[/font] text"], + ["[small]tiny[/small] text"], + ["[highlight]marked[/highlight] text"], + ["[u]underline[/u] text"], + ["[aname=top]anchor[/aname] text"], + ["[jumpto=top]jump[/jumpto] text"], + ["[color=red]a[/color] plain [color=blue]b[/color]"], + ["[center]\n\ncentered\n\n[/center]"], + ["[left]\n\nleft\n\n[/left]"], + ["[right]\n\nright\n\n[/right]"], + ["[indent]\n\nindented\n\n[/indent]"], + ["[ot]\n\naside\n\n[/ot]"], + ["[edit]\n\nnote\n\n[/edit]"], + ["[quote]\n\nquoted\n\n[/quote]"], + ["[list]\n[*]one\n[*]two\n[/list]"], + ["[ul]\n[*]one\n[*]two\n[/ul]"], + ["[ol]\n[*]one\n[*]two\n[/ol]"], + ["[list=1]\n[*]one\n[*]two\n[/list]"], + ["[list=a]\n[*]one\n[*]two\n[/list]"], + ["[list]\n[li]one[/li]\n[li]two[/li]\n[/list]"], + ["[list=a]\n[*]outer\n\n[list=a]\n[*]inner\n[/list]\n[/list]"], + ["[indent]\n\n[list]\n[*]indented item\n[/list]\n\n[/indent]"], + ["before\n\n[center]\n\nmiddle\n\n[/center]\n\nafter"], + ["text with an ![image](https://example.com/a.png)"], + ["[b]bold[/b] text", "**bold** text"], + ["[i]italic[/i] text", "*italic* text"], + ["[s]strike[/s] text", "~~strike~~ text"], + ["[url=https://example.com]link[/url]", "[link](https://example.com)"], + ["[code]\nraw [b]not bold[/b]\n[/code]", "```\nraw [b]not bold[/b]\n```"], + [ + "[color=red]red [b]and bold[/b][/color]", + "[color=red]red **and bold**[/color]", + ], + [ + "[b]bold [color=red]and red[/color][/b]", + "**bold [color=red]and red[/color]**", + ], + [ + "[size=200][b][i]all three[/i][/b][/size]", + "***[size=200]all three[/size]***", + ], + [ + "[center]\n\n[b]centered bold[/b]\n\n[/center]", + "[center]\n**centered bold**\n[/center]", + ], + [ + "[list]\n[*][b]bold item[/b]\n[*][color=red]red item[/color]\n[/list]", + "* **bold item**\n* [color=red]red item[/color]", + ], + [ + "a **markdown bold** and [b]bbcode bold[/b]", + "a **markdown bold** and **bbcode bold**", + ], +]; + +module( + "Integration | Component | prosemirror-editor - discourse-bbcode fidelity", + function (hooks) { + setupRenderingTest(hooks); + + hooks.beforeEach(async function () { + await resetRichEditorExtensions(); + registerRichEditorExtension(richEditorExtension); + }); + + CASES.forEach(([markdown, equivalent = markdown]) => { + test(`round trips ${JSON.stringify(markdown)}`, async function (assert) { + const [editorClass] = await setupRichEditor(assert, markdown); + + assert.strictEqual( + (await cook(editorClass.value)).toString(), + (await cook(equivalent)).toString(), + `cooked output should be unchanged, got source ${JSON.stringify(editorClass.value)}` + ); + }); + }); + } +); diff --git a/test/javascripts/integration/rich-editor-extension-test.js b/test/javascripts/integration/rich-editor-extension-test.js new file mode 100644 index 0000000..96531e6 --- /dev/null +++ b/test/javascripts/integration/rich-editor-extension-test.js @@ -0,0 +1,350 @@ +import { settled } from "@ember/test-helpers"; +import { module, test } from "qunit"; +import { + registerRichEditorExtension, + resetRichEditorExtensions, +} from "discourse/lib/composer/rich-editor-extensions"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; +import { setupRichEditor } from "discourse/tests/helpers/rich-editor-helper"; +import { i18n } from "discourse-i18n"; +import richEditorExtension from "discourse/plugins/discourse-bbcode/discourse/lib/rich-editor-extension"; + +// the browser occasionally reserializes style attributes on editor elements +// ("color:red" -> "color: red;", hex colors -> rgb()), so compare both sides +// in a canonical form +function normalizeStyles(html) { + return html.replace(/ style="([^"]*)"/g, (_, style) => { + const normalized = style + .replace(/\s*:\s*/g, ":") + .replace(/;\s*$/, "") + .replace(/"|["']/g, "") + .replace( + /rgb\((\d+),\s*(\d+),\s*(\d+)\)/g, + (__, r, g, b) => + "#" + + [r, g, b].map((n) => (+n).toString(16).padStart(2, "0")).join("") + ); + return ` style="${normalized}"`; + }); +} + +async function testMarkdown(assert, markdown, expectedHtml, expectedMarkdown) { + const [editorClass, html] = await setupRichEditor(assert, markdown); + + assert.strictEqual( + normalizeStyles(html), + normalizeStyles(expectedHtml), + `HTML should match for "${markdown}"` + ); + + assert.strictEqual( + editorClass.value, + expectedMarkdown, + `Markdown should match for "${markdown}"` + ); +} + +module( + "Integration | Component | prosemirror-editor - discourse-bbcode extension", + function (hooks) { + setupRenderingTest(hooks); + + hooks.beforeEach(async function () { + await resetRichEditorExtensions(); + registerRichEditorExtension(richEditorExtension); + }); + + Object.entries({ + size: [ + "Some [size=150]large[/size] text", + '

Some large text

', + "Some [size=150]large[/size] text", + ], + font: [ + "In [font=courier]monospace[/font] rendering", + "

In monospace rendering

", + "In [font=courier]monospace[/font] rendering", + ], + "font with spaces": [ + "In [font='Times New Roman']serif[/font] rendering", + "

In serif rendering

", + 'In [font="Times New Roman"]serif[/font] rendering', + ], + color: [ + "Some [color=red]red text[/color] here", + '

Some red text here

', + "Some [color=red]red text[/color] here", + ], + "hex color": [ + "Some [color=#eeff00]colored text[/color] here", + '

Some colored text here

', + "Some [color=#eeff00]colored text[/color] here", + ], + // pasting the same value is declined, since from a browser it means + // "no color" rather than an authored choice + "transparent color": [ + "Some [color=transparent]text[/color] here", + '

Some text here

', + "Some [color=transparent]text[/color] here", + ], + bgcolor: [ + "Some [bgcolor=yellow]highlighted text[/bgcolor] here", + '

Some highlighted text here

', + "Some [bgcolor=yellow]highlighted text[/bgcolor] here", + ], + highlight: [ + "Some [highlight]highlighted text[/highlight] here", + '

Some highlighted text here

', + "Some [highlight]highlighted text[/highlight] here", + ], + // a same-type tag with the same value adds nothing, so dropping it + // renders identically + "nested highlight": [ + "[highlight]outer [highlight]inner[/highlight] outer[/highlight]", + '

outer inner outer

', + "[highlight]outer inner outer[/highlight]", + ], + "nested identical color": [ + "[color=red]outer [color=red]inner[/color] outer[/color]", + '

outer inner outer

', + "[color=red]outer inner outer[/color]", + ], + "highlight nested in color": [ + "[color=red]a [highlight]b[/highlight] c[/color]", + '

a b c

', + "[color=red]a [highlight]b[/highlight] c[/color]", + ], + small: [ + "Some [small]tiny text[/small] here", + '

Some tiny text here

', + "Some [small]tiny text[/small] here", + ], + "nested inline bbcode": [ + "[color=red][size=200]both[/size][/color]", + '

both

', + "[size=200][color=red]both[/color][/size]", + ], + "sequential same-type marks": [ + "[color=red]outer[/color] [color=blue]inner[/color] [color=red]outer[/color]", + '

outer inner outer

', + "[color=red]outer[/color] [color=blue]inner[/color] [color=red]outer[/color]", + ], + aname: [ + "An [aname=target]anchor[/aname] here", + '

An anchor here

', + "An [aname=target]anchor[/aname] here", + ], + "aname with a bracket": [ + "[aname='a]b']anchor[/aname]", + '

anchor

', + '[aname="a]b"]anchor[/aname]', + ], + jumpto: [ + "A [jumpto=target]jump link[/jumpto] here", + '

A jump link here

', + "A [jumpto=target]jump link[/jumpto] here", + ], + "jumpto with a bracket": [ + "[jumpto='a]b']jump[/jumpto]", + '

jump

', + '[jumpto="a]b"]jump[/jumpto]', + ], + left: [ + "[left]\n\naligned left\n\n[/left]", + '

aligned left

', + "[left]\naligned left\n[/left]", + ], + center: [ + "[center]\n\naligned center\n\n[/center]", + '

aligned center

', + "[center]\naligned center\n[/center]", + ], + right: [ + "[right]\n\naligned right\n\n[/right]", + '

aligned right

', + "[right]\naligned right\n[/right]", + ], + indent: [ + "[indent]\n\nindented text\n\n[/indent]", + '

indented text

', + "[indent]\nindented text\n[/indent]", + ], + ot: [ + "[ot]\n\nan off-topic aside\n\n[/ot]", + '

an off-topic aside

', + "[ot]\nan off-topic aside\n[/ot]", + ], + edit: [ + "[edit]\n\nan edit note\n\n[/edit]", + '

an edit note

', + "[edit]\nan edit note\n[/edit]", + ], + "typed list": [ + "[list=1]\n[*]first\n[*]second\n[/list]", + '
  1. first

  2. second

', + "[list=1]\n[*]first\n[*]second\n[/list]", + ], + "alpha typed list": [ + "[list=a]\n[*]first\n[*]second\n[/list]", + '
  1. first

  2. second

', + "[list=a]\n[*]first\n[*]second\n[/list]", + ], + "typed list with a bracket": [ + "[list='a]b']\n[*]item\n[/list]", + '
  1. item

', + '[list="a]b"]\n[*]item\n[/list]', + ], + "plain list": [ + "[list]\n[*]first\n[*]second\n[/list]", + '
  • first

  • second

', + "* first\n* second", + ], + "li list items": [ + "[list]\n[li]first[/li]\n[li]second[/li]\n[/list]", + '
  • first

  • second

', + "* first\n* second", + ], + "uppercase tags": [ + "Some [COLOR=red]red text[/COLOR] here", + '

Some red text here

', + "Some [color=red]red text[/color] here", + ], + "ul list": [ + "[ul]\n[*]first\n[*]second\n[/ul]", + '
  • first

  • second

', + "* first\n* second", + ], + "ol list": [ + "[ol]\n[*]first\n[*]second\n[/ol]", + '
  1. first

  2. second

', + "1. first\n2. second", + ], + "markdown list still works": [ + "1. first\n2. second", + '
  1. first

  2. second

', + "1. first\n2. second", + ], + "markdown bullet list still works": [ + "* first\n* second", + '
  • first

  • second

', + "* first\n* second", + ], + }).forEach(([name, [markdown, html, expectedMarkdown]]) => { + test(name, async function (assert) { + await testMarkdown(assert, markdown, html, expectedMarkdown); + }); + }); + + // the editor's clipboard round-trips through toDOM/parseDOM, so pasted + // HTML must resolve to the bbcode nodes, not the more generic defaults + Object.entries({ + "pasted indent html": [ + '

indented

', + "[indent]\nindented\n[/indent]", + ], + "pasted typed list html": [ + '
  1. first

  2. second

', + "[list=a]\n[*]first\n[*]second\n[/list]", + ], + "pasted jumpto html": [ + '

go here

', + "go [jumpto=target]here[/jumpto]", + ], + "pasted edit html": [ + '

an edit note

', + "[edit]\nan edit note\n[/edit]", + ], + "pasted normalized font html": [ + '

serif

', + '[font="Times New Roman"]serif[/font]', + ], + "pasted bare font html": [ + '

mono

', + "[font=courier]mono[/font]", + ], + // the size and small marks both claim font-size, so the keyword has to + // fall past the percentage rule + "pasted x-small html": [ + '

tiny

', + "[small]tiny[/small]", + ], + "pasted rgb color html": [ + '

bright

', + "[color=#eeff00]bright[/color]", + ], + "pasted rgb bgcolor html": [ + '

marked

', + "[bgcolor=#ffff00]marked[/bgcolor]", + ], + "pasted normalized align html": [ + '

middle

', + "[center]\nmiddle\n[/center]", + ], + "pasted font stack html is not claimed": [ + '

stacked

', + "stacked", + ], + "pasted cooked edit html": [ + `
\n${i18n("bbcode.edit")}\n
\n
\n

an edit note

\n
`, + "[edit]\nan edit note\n[/edit]", + ], + "pasted cooked edit html from another locale is not mislabeled": [ + '
\nEditar:\n
\n
\n

an edit note

\n
', + "an edit note", + ], + "pasted transparent background html": [ + '

plain

', + "plain", + ], + "pasted unknown sepquote tag html": [ + '

note

', + "note", + ], + "pasted quoted aname html": [ + '

An anchor here

', + 'An [aname="O\'Brien x"]anchor[/aname] here', + ], + "pasted quoted jumpto html": [ + '

go here now

', + "go [jumpto=tar'get]here[/jumpto] now", + ], + "pasted quoted list type html": [ + '
  1. first

', + "[list=a'b]\n[*]first\n[/list]", + ], + "pasted spaced list type html": [ + '
  1. first

', + '[list="a b"]\n[*]first\n[/list]', + ], + }).forEach(([name, [html, expectedMarkdown]]) => { + test(name, async function (assert) { + const [editorClass] = await setupRichEditor(assert, ""); + + editorClass.view.pasteHTML(html); + await settled(); + + assert.strictEqual( + editorClass.value, + expectedMarkdown, + `Markdown should match for pasted "${html}"` + ); + }); + }); + + // content the editor can't represent exactly falls back to the markdown + // editor with its source untouched, rather than being rewritten: nested + // [size] compounds when cooked, so flattening it would resize the text + [ + "[color=red;position:fixed]unsafe[/color]", + "[font=bad!font]x[/font]", + "[color=red]outer [color=blue]inner[/color] outer[/color]", + "[size=200]outer [size=150]inner[/size] outer[/size]", + ].forEach((markdown) => { + test(`declines "${markdown}"`, async function (assert) { + await setupRichEditor(assert, markdown); + + assert.dom(".ProseMirror span").doesNotExist(); + }); + }); + } +); diff --git a/test/javascripts/lib/bbcode-cooked-test.js b/test/javascripts/lib/bbcode-cooked-test.js new file mode 100644 index 0000000..753e79d --- /dev/null +++ b/test/javascripts/lib/bbcode-cooked-test.js @@ -0,0 +1,21 @@ +import { setupTest } from "ember-qunit"; +import { module, test } from "qunit"; +import { cook } from "discourse/lib/text"; + +function cookedDocument(html) { + return new DOMParser().parseFromString(html, "text/html"); +} + +module("Unit | Lib | discourse-bbcode cooking", function (hooks) { + setupTest(hooks); + + test("sepquote tags include their structural type", async function (assert) { + const cooked = await cook("[edit]\nnote\n[/edit]"); + + assert.strictEqual( + cookedDocument(cooked).querySelector(".sepquote").dataset.tag, + "edit", + "the cooked block identifies the edit tag" + ); + }); +}); From 8b9141f70069ffa7bd198899d2f18b5bac9491c0 Mon Sep 17 00:00:00 2001 From: Renato Atilio Date: Thu, 6 Aug 2026 12:38:24 -0300 Subject: [PATCH 2/5] DEV: Address review of the rich editor extension Track whether an inline bbcode open token was ours, so its close ends the mark we opened rather than whichever one happened to be on top. The block wraps already did this; the inline path popped unconditionally. Build the cook sanitizer's allowlist and the editor's value checks from one set of charsets, so the two can't drift. The shared module sits under discourse-markdown/ because only that path is loaded into the server-side cooking context. Guard the parse state reads that assumed a node was open, and say in a comment why declining a token fails the parse. Assert that a declined post leaves the editor empty rather than that it renders no span, which a silently dropped tag would also satisfy, and drop the round trip cases that restate an exact round trip already asserted in the extension test. --- .../discourse/lib/rich-editor-extension.js | 94 ++++++++++--------- .../lib/discourse-markdown/bbcode-values.js | 14 +++ .../lib/discourse-markdown/bbcode.js | 20 +++- .../integration/bbcode-round-trip-test.js | 39 ++------ .../integration/rich-editor-extension-test.js | 4 +- 5 files changed, 87 insertions(+), 84 deletions(-) create mode 100644 assets/javascripts/lib/discourse-markdown/bbcode-values.js diff --git a/assets/javascripts/discourse/lib/rich-editor-extension.js b/assets/javascripts/discourse/lib/rich-editor-extension.js index 4c23d66..790e0e3 100644 --- a/assets/javascripts/discourse/lib/rich-editor-extension.js +++ b/assets/javascripts/discourse/lib/rich-editor-extension.js @@ -1,13 +1,15 @@ import { serializeBBCodeAttr } from "discourse/lib/text"; import { i18n } from "discourse-i18n"; +import { + ALIGNMENTS, + COLOR, + FONT, + SIZE, +} from "discourse/plugins/discourse-bbcode/lib/discourse-markdown/bbcode-values"; -const ALIGNMENTS = ["left", "right", "center"]; - -// the cook sanitizer's allowlist: a looser value would style the editor but -// be stripped from the rendered post -const SIZE_VALUE = /^\d{1,3}%$/; -const FONT_VALUE = /^[a-zA-Z0-9\s-]+$/; -const COLOR_VALUE = /^#?[a-zA-Z0-9]+$/; +const SIZE_VALUE = new RegExp(`^${SIZE}$`); +const FONT_VALUE = new RegExp(`^${FONT}$`); +const COLOR_VALUE = new RegExp(`^${COLOR}$`); // from a browser these mean "no color", unlike an authored [color=transparent] const NON_COLORS = [ @@ -76,6 +78,8 @@ const SPAN_MARKS = new Map([ ], ]); +const INLINE_TAGS = ["span", "a"]; + function inlineMarkFor(token, schema) { if (token.tag === "span") { const [, property, value] = @@ -105,32 +109,46 @@ function serializableAttr(value) { return value && !value.includes("\n") ? value : null; } -// a mark set holds one per type: an identical nesting adds nothing, a differing -// one can't be represented, so it's declined and the post stays in markdown +// every open we see pushes an entry, so the matching close knows whether it was +// ours: a mark we opened, null for one we swallowed, false for one we passed on +// to another extension. without that an unclaimed open would leave its close to +// end whichever mark happened to be on top. function openInlineMark(state, mark) { const open = (state.bbcodeInlineMarks ??= []); - const enclosing = open.find((entry) => entry?.type === mark.type); - - if (enclosing) { - if (!enclosing.eq(mark)) { - return false; - } + const enclosing = mark && open.find((entry) => entry?.type === mark.type); + + // a mark set holds one per type: an identical nesting adds nothing, a + // differing one can't be represented. declining leaves the token to the other + // bbcode_open handlers, and with none of them claiming it the parse fails and + // the post stays in the markdown editor with its source intact. + if (!mark || (enclosing && !enclosing.eq(mark))) { + open.push(false); + return false; + } - // balances the matching close - open.push(null); - return true; + if (!enclosing) { + state.openMark(mark); } - state.openMark(mark); - open.push(mark); + open.push(enclosing ? null : mark); return true; } function closeInlineMark(state) { + if (!state.bbcodeInlineMarks?.length) { + return false; + } + const mark = state.bbcodeInlineMarks.pop(); if (mark) { state.closeMark(mark); } + + return mark !== false; +} + +function inSepquote(state) { + return state.top()?.type.name === "bbcode_sepquote"; } function wrapInTag(state, node, tag) { @@ -324,18 +342,14 @@ const extension = { parse: { bbcode_open(state, token) { - const mark = inlineMarkFor(token, state.schema); - return !!mark && openInlineMark(state, mark); + return ( + INLINE_TAGS.includes(token.tag) && + openInlineMark(state, inlineMarkFor(token, state.schema)) + ); }, bbcode_close(state, token) { - if ( - (token.tag === "span" || token.tag === "a") && - state.bbcodeInlineMarks?.length - ) { - closeInlineMark(state); - return true; - } + return INLINE_TAGS.includes(token.tag) && closeInlineMark(state); }, bbcode_highlight_open(state) { @@ -346,10 +360,7 @@ const extension = { }, bbcode_highlight_close(state) { - if (state.bbcodeInlineMarks?.length) { - closeInlineMark(state); - return true; - } + return closeInlineMark(state); }, // shared with any wrapping block bbcode tag, so track which opens were ours @@ -401,31 +412,22 @@ const extension = { }, sepquote_close(state) { - if (state.top().type.name === "bbcode_sepquote") { + if (inSepquote(state)) { state.closeNode(); return true; } }, span_open(state, token) { - if ( - token.attrGet("class") === "smallfont" && - state.top().type.name === "bbcode_sepquote" - ) { - return true; - } + return token.attrGet("class") === "smallfont" && inSepquote(state); }, span_close(state) { - if (state.top().type.name === "bbcode_sepquote") { - return true; - } + return inSepquote(state); }, soft_break(state) { - if (state.top().type.name === "bbcode_sepquote") { - return true; - } + return inSepquote(state); }, bbcode_list: { diff --git a/assets/javascripts/lib/discourse-markdown/bbcode-values.js b/assets/javascripts/lib/discourse-markdown/bbcode-values.js new file mode 100644 index 0000000..080f9ee --- /dev/null +++ b/assets/javascripts/lib/discourse-markdown/bbcode-values.js @@ -0,0 +1,14 @@ +// the charsets a tag value may use. the cook sanitizer builds its allowlist +// from these, and the rich editor declines anything looser, so the editor can +// never show styling the rendered post drops. +// +// lives under discourse-markdown/ because only that path is loaded into the +// server-side cooking context; exporting no setup keeps it out of the feature list. + +export const SIZE = "\\d{1,3}%"; +export const ABSOLUTE_SIZE = + "xx-small|x-small|small|medium|large|x-large|xx-large"; +export const COLOR = "#?[a-zA-Z0-9]+"; +export const FONT = "[a-zA-Z0-9\\s-]+"; + +export const ALIGNMENTS = ["left", "right", "center"]; diff --git a/assets/javascripts/lib/discourse-markdown/bbcode.js b/assets/javascripts/lib/discourse-markdown/bbcode.js index 23b304e..f74daca 100644 --- a/assets/javascripts/lib/discourse-markdown/bbcode.js +++ b/assets/javascripts/lib/discourse-markdown/bbcode.js @@ -1,4 +1,16 @@ import { i18n } from "discourse-i18n"; +import { + ABSOLUTE_SIZE, + ALIGNMENTS, + COLOR, + FONT, + SIZE, +} from "discourse/plugins/discourse-bbcode/lib/discourse-markdown/bbcode-values"; + +const SPAN_STYLE = new RegExp( + `^(font-size:(${ABSOLUTE_SIZE}|${SIZE})|background-color:${COLOR}|color:${COLOR}|font-family:'${FONT}')$` +); +const DIV_STYLE = new RegExp(`^text-align:(${ALIGNMENTS.join("|")})$`); function wrap(tag, attr, callback) { return function (startToken, finishToken, tagInfo) { @@ -80,7 +92,7 @@ function setupMarkdownIt(md) { wrap: wrap("a", "href", (tagInfo) => "#" + tagInfo.attrs._default), }); - ["left", "right", "center"].forEach((dir) => { + ALIGNMENTS.forEach((dir) => { md.block.bbcode.ruler.push(dir, { tag: dir, wrap: function (token) { @@ -216,13 +228,11 @@ export function setup(helper) { helper.allowList({ custom(tag, name, value) { if (tag === "span" && name === "style") { - return /^(font-size:(xx-small|x-small|small|medium|large|x-large|xx-large|[0-9]{1,3}%)|background-color:#?[a-zA-Z0-9]+|color:#?[a-zA-Z0-9]+|font-family:'[a-zA-Z0-9\s-]+')$/.exec( - value - ); + return SPAN_STYLE.exec(value); } if (tag === "div" && name === "style") { - return /^text-align:(center|left|right)$/.exec(value); + return DIV_STYLE.exec(value); } }, }); diff --git a/test/javascripts/integration/bbcode-round-trip-test.js b/test/javascripts/integration/bbcode-round-trip-test.js index 58b19aa..976d61d 100644 --- a/test/javascripts/integration/bbcode-round-trip-test.js +++ b/test/javascripts/integration/bbcode-round-trip-test.js @@ -8,43 +8,18 @@ import { setupRenderingTest } from "discourse/tests/helpers/component-test"; import { setupRichEditor } from "discourse/tests/helpers/rich-editor-helper"; import richEditorExtension from "discourse/plugins/discourse-bbcode/discourse/lib/rich-editor-extension"; -// the source may be normalized, but it has to keep cooking to the same post. -// the second entry is the equivalent source for tags the editor rewrites to -// markdown, which cook renders with the same styling under other tags. +// sources the editor rewrites: what it writes back has to keep cooking to the +// same post. the second entry is the expected equivalent, which cook renders +// with the same styling under other tags. sources that round trip byte for byte +// belong in rich-editor-extension-test, which asserts them exactly. const CASES = [ - ["[color=red]red[/color] text"], - ["[color=#ff0000]hex[/color] text"], - ["[bgcolor=yellow]marked[/bgcolor] text"], - ["[size=150]large[/size] text"], - ["[font=courier]mono[/font] text"], - ["[small]tiny[/small] text"], - ["[highlight]marked[/highlight] text"], - ["[u]underline[/u] text"], - ["[aname=top]anchor[/aname] text"], - ["[jumpto=top]jump[/jumpto] text"], - ["[color=red]a[/color] plain [color=blue]b[/color]"], - ["[center]\n\ncentered\n\n[/center]"], - ["[left]\n\nleft\n\n[/left]"], - ["[right]\n\nright\n\n[/right]"], - ["[indent]\n\nindented\n\n[/indent]"], - ["[ot]\n\naside\n\n[/ot]"], - ["[edit]\n\nnote\n\n[/edit]"], - ["[quote]\n\nquoted\n\n[/quote]"], + ["[list=a]\n[*]outer\n\n[list=a]\n[*]inner\n[/list]\n[/list]"], + ["[indent]\n\n[list]\n[*]indented item\n[/list]\n\n[/indent]"], + ["before\n\n[center]\n\nmiddle\n\n[/center]\n\nafter"], ["[list]\n[*]one\n[*]two\n[/list]"], ["[ul]\n[*]one\n[*]two\n[/ul]"], ["[ol]\n[*]one\n[*]two\n[/ol]"], - ["[list=1]\n[*]one\n[*]two\n[/list]"], - ["[list=a]\n[*]one\n[*]two\n[/list]"], ["[list]\n[li]one[/li]\n[li]two[/li]\n[/list]"], - ["[list=a]\n[*]outer\n\n[list=a]\n[*]inner\n[/list]\n[/list]"], - ["[indent]\n\n[list]\n[*]indented item\n[/list]\n\n[/indent]"], - ["before\n\n[center]\n\nmiddle\n\n[/center]\n\nafter"], - ["text with an ![image](https://example.com/a.png)"], - ["[b]bold[/b] text", "**bold** text"], - ["[i]italic[/i] text", "*italic* text"], - ["[s]strike[/s] text", "~~strike~~ text"], - ["[url=https://example.com]link[/url]", "[link](https://example.com)"], - ["[code]\nraw [b]not bold[/b]\n[/code]", "```\nraw [b]not bold[/b]\n```"], [ "[color=red]red [b]and bold[/b][/color]", "[color=red]red **and bold**[/color]", diff --git a/test/javascripts/integration/rich-editor-extension-test.js b/test/javascripts/integration/rich-editor-extension-test.js index 96531e6..9199633 100644 --- a/test/javascripts/integration/rich-editor-extension-test.js +++ b/test/javascripts/integration/rich-editor-extension-test.js @@ -343,7 +343,9 @@ module( test(`declines "${markdown}"`, async function (assert) { await setupRichEditor(assert, markdown); - assert.dom(".ProseMirror span").doesNotExist(); + // a declined parse leaves nothing behind and hands the post back to the + // markdown editor. dropping the tag instead would render its content. + assert.dom(".ProseMirror").hasNoText(); }); }); } From cf1fc9d97269b431a46a2f8a95361fe236739649 Mon Sep 17 00:00:00 2001 From: Renato Atilio Date: Thu, 6 Aug 2026 14:22:18 -0300 Subject: [PATCH 3/5] FIX: Decline attribute values that can't be written back A value needing quotes that contains every supported delimiter leaves the serializer no pair to wrap it in, and its fallback strips the double quotes. Round trip the value through the serializer and decline it when it doesn't survive, rather than duplicating the delimiter list here. The fidelity tests compared the cooked output of the editor's value against the cooked source, which a declined parse satisfies on its own: the editor is left empty and the value keeps the original markdown. Assert the editor rendered something first. --- .../discourse/lib/rich-editor-extension.js | 11 +++++++++-- .../javascripts/integration/bbcode-round-trip-test.js | 4 ++++ .../integration/rich-editor-extension-test.js | 6 ++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/assets/javascripts/discourse/lib/rich-editor-extension.js b/assets/javascripts/discourse/lib/rich-editor-extension.js index 790e0e3..c3230bf 100644 --- a/assets/javascripts/discourse/lib/rich-editor-extension.js +++ b/assets/javascripts/discourse/lib/rich-editor-extension.js @@ -104,9 +104,16 @@ function inlineMarkFor(token, schema) { return null; } -// a bbcode tag is a single line, so no quoting can hold a newline +// a bbcode tag is a single line, so no quoting can hold a newline. a value +// needing quotes that leaves no quote pair unused loses its double quotes to +// the serializer's fallback, so accept only what survives the round trip function serializableAttr(value) { - return value && !value.includes("\n") ? value : null; + if (!value || value.includes("\n")) { + return null; + } + + // the attribute name plays no part in how the value is quoted + return serializeBBCodeAttr(value, "attr").includes(value) ? value : null; } // every open we see pushes an entry, so the matching close knows whether it was diff --git a/test/javascripts/integration/bbcode-round-trip-test.js b/test/javascripts/integration/bbcode-round-trip-test.js index 976d61d..122bcd2 100644 --- a/test/javascripts/integration/bbcode-round-trip-test.js +++ b/test/javascripts/integration/bbcode-round-trip-test.js @@ -60,6 +60,10 @@ module( test(`round trips ${JSON.stringify(markdown)}`, async function (assert) { const [editorClass] = await setupRichEditor(assert, markdown); + // a declined parse leaves the editor empty and the source untouched, + // which would satisfy the comparison below on its own + assert.dom(".ProseMirror").hasAnyText(); + assert.strictEqual( (await cook(editorClass.value)).toString(), (await cook(equivalent)).toString(), diff --git a/test/javascripts/integration/rich-editor-extension-test.js b/test/javascripts/integration/rich-editor-extension-test.js index 9199633..d6cc1a3 100644 --- a/test/javascripts/integration/rich-editor-extension-test.js +++ b/test/javascripts/integration/rich-editor-extension-test.js @@ -304,6 +304,12 @@ module( '

An anchor here

', 'An [aname="O\'Brien x"]anchor[/aname] here', ], + // a value needing quotes that uses every delimiter leaves the serializer + // no pair to wrap it in, and its fallback drops the double quotes + "pasted aname that can't be quoted losslessly": [ + `

An anchor here

`, + "An anchor here", + ], "pasted quoted jumpto html": [ '

go here now

', "go [jumpto=tar'get]here[/jumpto] now", From c4557467fc3d37447410d1c3339363ebd2974335 Mon Sep 17 00:00:00 2001 From: Renato Atilio Date: Thu, 6 Aug 2026 15:27:59 -0300 Subject: [PATCH 4/5] FIX: Compare an attribute value against what parses back A substring check on the written tag accepts a value whose own trailing quote was stripped, because the wrapper's closing quote lands in the same place and makes the tag look like it still holds the value. Parse the written tag back with core's own parser and require the value to match, so both halves of the round trip are the canonical ones. The regression case puts the quote at the boundary, where the substring check passed. --- assets/javascripts/discourse/lib/rich-editor-extension.js | 6 ++++-- test/javascripts/integration/rich-editor-extension-test.js | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/assets/javascripts/discourse/lib/rich-editor-extension.js b/assets/javascripts/discourse/lib/rich-editor-extension.js index c3230bf..9243434 100644 --- a/assets/javascripts/discourse/lib/rich-editor-extension.js +++ b/assets/javascripts/discourse/lib/rich-editor-extension.js @@ -1,4 +1,5 @@ import { serializeBBCodeAttr } from "discourse/lib/text"; +import { parseAttributesString } from "discourse/lib/wrap-utils"; import { i18n } from "discourse-i18n"; import { ALIGNMENTS, @@ -106,14 +107,15 @@ function inlineMarkFor(token, schema) { // a bbcode tag is a single line, so no quoting can hold a newline. a value // needing quotes that leaves no quote pair unused loses its double quotes to -// the serializer's fallback, so accept only what survives the round trip +// the serializer's fallback, so accept only what parses back to itself function serializableAttr(value) { if (!value || value.includes("\n")) { return null; } // the attribute name plays no part in how the value is quoted - return serializeBBCodeAttr(value, "attr").includes(value) ? value : null; + const written = serializeBBCodeAttr(value, "attr"); + return parseAttributesString(written).attr === value ? value : null; } // every open we see pushes an entry, so the matching close knows whether it was diff --git a/test/javascripts/integration/rich-editor-extension-test.js b/test/javascripts/integration/rich-editor-extension-test.js index d6cc1a3..aea5d84 100644 --- a/test/javascripts/integration/rich-editor-extension-test.js +++ b/test/javascripts/integration/rich-editor-extension-test.js @@ -305,9 +305,11 @@ module( 'An [aname="O\'Brien x"]anchor[/aname] here', ], // a value needing quotes that uses every delimiter leaves the serializer - // no pair to wrap it in, and its fallback drops the double quotes + // no pair to wrap it in, and its fallback drops the double quotes. the + // trailing one is the boundary: the wrapper's closing quote lands where + // it was, so the written tag still looks like it holds the value "pasted aname that can't be quoted losslessly": [ - `

An anchor here

`, + `

An anchor here

`, "An anchor here", ], "pasted quoted jumpto html": [ From 4cb8270d21fe5e5152fbc1162137b9b53fc23b2c Mon Sep 17 00:00:00 2001 From: Renato Atilio Date: Thu, 6 Aug 2026 15:43:19 -0300 Subject: [PATCH 5/5] DEV: Describe the fidelity precondition assertion --- test/javascripts/integration/bbcode-round-trip-test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/javascripts/integration/bbcode-round-trip-test.js b/test/javascripts/integration/bbcode-round-trip-test.js index 122bcd2..e65889e 100644 --- a/test/javascripts/integration/bbcode-round-trip-test.js +++ b/test/javascripts/integration/bbcode-round-trip-test.js @@ -60,9 +60,11 @@ module( test(`round trips ${JSON.stringify(markdown)}`, async function (assert) { const [editorClass] = await setupRichEditor(assert, markdown); - // a declined parse leaves the editor empty and the source untouched, - // which would satisfy the comparison below on its own - assert.dom(".ProseMirror").hasAnyText(); + // a declined parse keeps the original source in the value, so the + // comparison below would pass with nothing having been parsed + assert + .dom(".ProseMirror") + .hasAnyText("the editor parsed the post rather than declining it"); assert.strictEqual( (await cook(editorClass.value)).toString(),