Add colour and underline buttons to the Markdown editor

[paul999]

Colour and underline buttons for the Markdown editor, so new knowledge base articles can use the styling migrated articles keep (issue #19 in paul999/phpbb-website-private). Follows #33, which added the CSS classes.

What authors get

• U (Ctrl/Cmd+U) underlines the selection, or takes the underline off again, also under a colour.
• A opens a menu with one entry per palette colour, each shown in its own colour, plus "Remove colour". A new colour replaces the old one rather than nesting.
• It works with several cursors, and the selection stays on the styled words, so red then underline acts on the same text.
• The buttons only appear when the website asks for them (data-markdown-styles on the textarea, from MarkdownEditorType's text_styles option). The list of classes comes from the website, from the same constants its renderer checks, so this script keeps no copy of the palette. Team blurbs don't get the buttons, because the team page doesn't style these classes.

Why the JS is bigger than two buttons

The website keeps colour and underline as <span class="text-…">. SafeMarkdown renders such a span only when it pairs up within one block without crossing other formatting, and leaves it off the page otherwise (phpbb/phpbb-website-private#222). So the buttons try every edit before making it:

• They work line by line, and leave code blocks (in quotes and list items too), fences, tables, rules, setext underlines and reference definitions alone. List, heading and quote markers stay outside the span.
• Each edit is tried first: the paragraph is rendered with a probe span and without it, using EasyMDE's own renderer. marked's raw HTML is taken through the sanitizerFunction hook, unchanged, so the preview is unaffected. The edit is made only if:
  • the probe holds properly nested inline content with no block in it,
  • taking the probe out gives back exactly the original rendering, and
  • by CommonMark's own rules (the site's), neither edge falls inside inline code, splits a backtick run, follows a backslash or crosses a link's brackets.
• If the selected part fails, the line's whole text is tried once. If that fails too, the line is left alone.

Testing

• The whole path works end to end on the website's Markdown demo page: the buttons, the editor preview and the server render.
• Four review rounds, using a puppeteer harness that drives the real EasyMDE and renders every result through the site's SafeMarkdown:
  • 202 cases of realistic content and edge cases all come out right, including headings, lists, quotes, code, links, reference links, hard breaks, multiple cursors and recolouring.
  • About 34,000 randomly generated edits left no span tag showing as HTML text and caused no page errors.
  • What still gets flagged is delimiter soup (like ****]***) where marked and CommonMark disagree about emphasis. On the site the worst outcome there is a dropped colour.

After this merges, composer update phpbb/website-assets goes into phpbb/phpbb-website-private#222.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

The JavaScript contains a critical sanitizer issue and several moderate Markdown-editing issues that must be addressed.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds optional colour and underline controls to the EasyMDE Markdown editor, with Markdown validation and new toolbar styling.

Changes:

• Adds data-driven colour and underline editing.
• Adds selection handling, shortcuts, and rendering validation.
• Styles the new toolbar controls and colour menu.

File summaries

File	Review findings
js/markdown-editor.js	Critical, 1 vote (line 98): The sanitizer hook handling prevents probe detection and causes edits to be rejected. Moderate, 1 vote (lines 413, 419, 433, 543, 741, 996): Preserve default shortcuts; avoid treating escaped/code text as style spans; handle even backslash runs in tables; detect headings after list/quote prefixes; correctly identify reference continuations; and expand containing spans for partial selections.
css/markdown-editor.css	No final review comments.

Review details

Suppressed comments (6)

js/markdown-editor.js:996

• EasyMDE treats shortcuts as the complete shortcut map, so this replaces its default bindings: editors without styles get {}, and styled editors retain only underline. That disables the existing Cmd/Ctrl-B, Cmd/Ctrl-I, Cmd/Ctrl-K and other built-in keyboard shortcuts on every editor. Preserve the default map and add the underline binding instead.

			shortcuts: styles && styles.underline ? { underline: 'Cmd-U' } : {},

js/markdown-editor.js:547

• spanPairs parses raw Markdown rather than rendered HTML, so tag-shaped text inside an inline code span or escaped HTML is treated as an existing style. Recolouring such a selection removes those literal <span ...> and </span> characters before rendersCleanly runs; the probe then compares the already-mutated body and accepts the edit, silently changing the code/text. Only remove pairs that are actual rendered style spans, excluding code and escaped text.

			cuts.slice().sort(function (a, b) {
				return b[0] - a[0];
			}).forEach(function (cut) {
				body = body.slice(0, cut[0] - start) + body.slice(cut[0] - start + cut[1]);
			});

js/markdown-editor.js:420

• This table check treats a pipe as escaped whenever the immediately preceding character is a backslash. In CommonMark an even run is not an escape, so a row such as a\\|b can be parsed as a table while this code styles it, contrary to the stated rule that table rows remain untouched. Count consecutive backslashes modulo two.

		if (/(^|[^\\])\|/.test(text.replace(/(`+)[^`]*?\1/g, ''))) {
			return true;

js/markdown-editor.js:439

• The closing-hash check only recognizes headings that start directly with #. For > # Heading # or - # Heading #, BLOCK_PREFIX removes the quote/list marker but this check fails, so a whole-line selection includes the closing hashes in the probe; the render equality check then rejects an otherwise valid edit. Detect an ATX heading after optional list/quote prefixes so the closing hashes remain outside the span.

		if (/^ {0,3}#{1,6}(\s|$)/.test(text)) {
			var closing = text.match(/\s+#+\s*$/);

			if (closing) {
				end = closing.index;
			}
		}

js/markdown-editor.js:415

• This treats every nonblank line immediately after a reference definition as a reference continuation. A valid document such as [id]: /url followed directly by Paragraph makes that paragraph ordinary Markdown, but this condition marks it untouchable so it can never be styled or unstyled. Restrict the check to a syntactically valid continuation title (or parse the definition and continuation together).

		if (line > 0 && REFERENCE.test(cm.getLine(line - 1)) && !isBlank(text)) {
			return true;
		}

js/markdown-editor.js:750

• This only expands a selection when the opening and closing tags are immediately adjacent to its edges. Selecting a proper subset of text inside an existing colour span therefore leaves the selection inside the old span: recolouring nests the new span instead of replacing the old one, and “Remove colour” finds no tag overlapping the selection and does nothing. Expand/split containing target spans before applying the edit so partial selections have the same replace/remove behavior as selections restored by the toolbar.

		for (;;) {
			var opening = before.match(/<span class="[^"<>]*">$/);
			var closing = after.match(/^<\/span>/);

			if (!opening || !closing) {
				break;
			}

			from = { line: from.line, ch: from.ch - opening[0].length };
			to = { line: to.line, ch: to.ch + closing[0].length };

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.