Markdown syntax for XWiki Group blocks in the Markdown extension

Hi devs,

I’d like to present the solution I’ve chosen for MARKDOWN-56 (“Add support for Group syntax”) in the Markdown Syntax extension. It’s already implemented and pushed (PR #7), but it hasn’t been released yet, so we can still change it. I’d like your feedback before the next release of the extension.

The problem

Markdown (CommonMark) has no equivalent of the XWiki Group block ((% class="..." %)(((...))) in XWiki Syntax 2.1). Until now, the Markdown renderer silently dropped Groups and their parameters. For example, an info box inserted with the WYSIWYG editor’s Styles dropdown (class="box infomessage") disappeared when saving a Markdown page.

Proposed syntax

A Group is written as a <div> that has a data-xwiki-group marker attribute, with the Group parameters as HTML attributes:

<div data-xwiki-group class="box infomessage">

This is an **info** box with:

* a list
* and a [link](https://www.xwiki.org)

</div>

This is equivalent to:

(% class="box infomessage" %)
(((
This is an **info** box with:

* a list
* and a [[link>>https://www.xwiki.org]]
)))

The opening and closing tags are each alone on their line and surrounded by blank lines. That’s valid CommonMark, so GitHub and other Markdown tools display it correctly. The blank lines are required: in CommonMark, an HTML block starting with <div only ends at a blank line, and its content is raw HTML. Without the blank line after the opening tag, the Group content wouldn’t be parsed as Markdown. The marker only identifies the Group after parsing, and doesn’t change how the Markdown is parsed.

Both directions are supported: the renderer produces this syntax for any Group, and the parser turns it back into a Group. This means Groups survive the WYSIWYG editor round trip. It works for both markdown/1.2 and markdown+github/1.0.

Why a marker attribute?

To not break backward compatibility. Existing Markdown pages can already contain hand-written <div> elements, which are parsed as HTML macros. Converting every <div> into a Group would change the XDOM of that content. With the marker, only a <div data-xwiki-group> that is alone in its HTML block and has a matching </div> at the same level becomes a Group. Any other <div> is still parsed exactly as before.

Alternatives considered

  • Convert the whole Group to HTML: the Group content (lists, tables, etc.) would no longer be Markdown, which defeats the purpose of using Markdown.
  • Pandoc-style fenced divs (::: {.box .infomessage} … :::): readable, but not CommonMark (other Markdown tools would show it as text), and flexmark doesn’t support it, so we’d need to write and maintain our own flexmark extension.
  • Treat every <div> as a Group, possibly in a new syntax version or behind a configuration flag: this either changes the meaning of existing content, or doesn’t fix the WYSIWYG editor use case by default.

Details and limitations

  • Every Group is rendered with a <div>, even without parameters, for an exact round trip. The only exception is the parameterless Group that the parser itself creates around an indented code block in a list item, so that the output of existing content doesn’t change.
  • Groups can be nested. Inside a list item, a Group is indented like the rest of the list item content.
  • As in XWiki Syntax 2.1, a Group is always a block: a Group inside a paragraph is rendered on its own lines.
  • Markdown tables can’t contain blocks, so a Group in a table cell is still rendered without its <div> (same as before).
  • Parameter values are HTML-escaped. Parameters whose names aren’t valid HTML attribute names can’t be expressed and are dropped.

Questions

  1. Are you OK with this syntax, and more specifically with the data-xwiki-group marker name? An alternative would be data-xwiki-type="group", which could later be reused for other XWiki-specific blocks.
  2. Do you see any problem with the backward compatibility choice (unmarked <div> elements staying HTML macros)?

Thanks

I’m wondering if it is really necessary for backwards compatibility to add the marker attribute, I think I would be more in favor of converting all <div> to group blocks as that corresponds to what the HTML parser/renderer do. It feels weird to have such differences between HTML inside Markdown and plain HTML. The justification that it is not really a breakage would be that the HTML output of both versions is identical, so it’s not actually changing the way pages that contain <div> are displayed, it just changes the intermediate representation in the XDOM (and the WYSIWYG editor).

Thanks @MichaelHamann. I’ve checked your points against the code.

Consistency with the HTML parser/renderer: agreed, the XHTML parser converts a <div> into a Group and the XHTML renderer outputs a Group as a <div>. Note however that even without the marker, only the “block” form of a <div> (opening and closing tags alone in their HTML block, surrounded by blank lines, balanced at the same level) could become a Group. Something like <div>text</div> on a single line, or a <div> followed by HTML without a blank line, would still be an HTML macro, since CommonMark keeps it as a single raw HTML block. So the difference with plain HTML would be reduced, not removed.

Identical HTML output: it’s only true when the author has script right. Since 14.7, XWikiHTMLRawBlockFilter makes the HTML macro restricted when the author doesn’t have script right, and in restricted mode HTMLRawBlockFilter cleans the HTML even with clean="false". Currently, a <div class="box infomessage"> in Markdown generates 2 separate HTML macros (<div ...> and </div>), each cleaned independently. From reading the code, I expect the first one to become an empty <div class="box infomessage"></div> and the second one to be dropped, which means the content isn’t inside the box (I haven’t tried it on a running instance yet). With a Group, the <div> would wrap the content properly. So for authors without script right, the output would change, but in a good way. This goes in the direction of your proposal.

A difference you didn’t mention: section editing. XWikiDocument.getFilteredHeaders() only supports editing top-level sections, so headings located inside a Group can’t be section-edited. Currently, headings located between the two HTML macros are top-level and thus section-editable. If all <div>s become Groups, existing pages with headings inside a <div> would lose these section edit links. It’s the same behavior as for (((...))) in XWiki Syntax 2.1, but it’s still a change for existing content.

Security, still to be checked: without the marker, the attributes of any hand-written <div> would become Group parameters rendered by the XHTML renderer, instead of going through the HTML macro’s restricted cleaning. I believe recent XWiki versions sanitize these attributes (e.g. onclick) in the XHTML renderer too, but I still need to verify it.

So I’m OK to go with your proposal (no marker, all block <div>s become Groups) if we accept the section editing change and once the security point is verified. The implementation change is small (stop requiring and generating the marker). WDYT? Others?

Given it matches XWiki Syntax 2.1 , then it sounds alright to me. It would be interesting to know if users actually relied on it to work around this limitation.

The idea of this data-xwiki-type marker is interesting and I wonder if it could be used for other improvements of the MD syntax extension or some refactoring to simplify the resolution of some complex situations.


Overall I’m +1 for avoiding markers for now.

Thanks!

Based on how I implemented the sanitizer, both should go through exactly the same code, if there is any difference in safety, it would be a security vulnerability. The only difference would be that the HTML renderer tries to prefix unsafe attributes (to avoid losing them through WYSIWYG editing, the prefix is removed again when parsing the HTML) while the cleaning removes them, relying on the macro metadata for recovering them from the WYSIWYG editor.

I’ve tested it on XWiki 18.7.0, with a <div class="box infomessage" onclick="alert(1)" style="color:red"> wrapping a heading and a paragraph, saved once by an author with script right and once by an author without it. I compared a Markdown <div> (current HTML macros) with a Group:

Author with script right Author without script right
Markdown <div> (HTML macros) box wraps the content, onclick kept empty box, content outside it, onclick removed
Group box wraps the content, onclick prefixed (data-xwiki-translated-attribute-onclick) same

So:

  • Security: confirmed, the Group is sanitized too (as @MichaelHamann explained).
  • Authors without script right: the Group fixes the currently broken rendering.
  • Section editing: confirmed, headings inside the Group are not section-editable anymore.
  • One more difference: for authors with script right, the HTML macro isn’t sanitized, so event handlers (onclick, etc.) in a hand-written <div> currently work, and they would stop working if the <div> became a Group. I think that’s an acceptable (and rare) change, but it’s a behavior change for existing content.

Since Lucas is also +1, unless someone objects, I’ll go without the marker.

It’s now been implemented. See: