Page MenuHomePhabricator

Chart output makes VE support difficult
Open, MediumPublic

Description

Since T376971 the chart extension outputs a custom <wiki-chart> tag, unless there is an error (e.g. missing data URL) in which case it outputs a plain <div> with error styles.

The VE converter can match tags by:

  1. tag name (e.g. <figure>, <h1>)
  2. RDFa type (e.g. typeof="mw:Extension/math)
  3. generic match function

As the tag is generated as a parser function we also get <wiki-chart typeof="mw:Transclusion"> (or <div typeof="mw:Transclusion"> in the error state).

There are a couple of problems here:

  1. The only way to match all these cases is by using a custom match function. The match function needs to JSON-decode the data-mw value and look for parts[0].template.target.function. We end up doing this check on every template node in the document, which is pretty inefficient.
  2. The above could be resolved by using <wiki-chart> for error messages, but the custom tag is unwrapped by our security library DOMPurify when pasting a chart from one VE instance to another. We could possibly allow-list the tag, but I don't think we can (or want to) generically allow-list unknown tags for other extensions that might take this approach in the future.

I would therefore suggest we

  1. Avoid custom tags, presumably there is nothing we can do with these that we can't do with classes or other attributes.
  2. Find a way for Parsoid to add the name of the generating function to the typeof attribute or somewhere more accessible than the data-mw JSON

Details

Event Timeline

Esanders renamed this task from Chart output make VE support difficult to Chart output makes VE support difficult.May 4 2025, 8:55 AM

Avoid custom tags, presumably there is nothing we can do with these

<wiki-chart> triggers the web extension class WikiChart extends HTMLElement to (re-)render the chart as an interactive chart: https://github.com/wikimedia/mediawiki-extensions-Chart/blob/master/resources/ext.chart/bootstrap.js

Jdlrobson-WMF added subscribers: cscott, santhosh.

Avoid custom tags, presumably there is nothing we can do with these that we can't do with classes or other attributes.

The use of web components simplifies a lot of the JavaScript rendering and has lots of benefits. If Parsoid doesn't support this out of the box I would recommend we attempt to fix that first so that our HTML can make use of modern web components.

Find a way for Parsoid to add the name of the generating function to the typeof attribute or somewhere more accessible than the data-mw JSON

@cscott do you have a sense of how difficult this would be?

We should probably convert this to the "v3 parser function" output (T373253/T391702) which should apply a more useful typeof (mw:ParserFunction/chart) solving the match problem.

I don't have a strong opinion about custom tags, other than noting that (a) we tried this one before with <figure-inline> and decided it was a bad idea and had to convert it back (T251641, but 5 years have passed, maybe things are different), and (b) if we're using client-side JS already we can easily convert <div typeof="mw:ParserFunction/chart"> to <wiki-chart> client-side, can't we?

Switching to use the new "v3 parser function" output requires writing a Parsoid fragment handler, which is probably blocked (for now) on T390344: v3 parserfunction serialization doesn't properly support named arguments, but that task is expected to be fixed by the (performance-motivated) work on T393391: Refactor PEG grammar for transclusions. So we could probably find time to get Charts moved over to a fragment handler by (say) the end of the quarter, if we agree that's a direction we'd like to move.

If be interested to hear what these benefits are, because custom tags have caused problems for both parsing and editing teams. We have lots of other dynamically infused content on the page and the wikipage.content is the standard way to support this.

If we think that we should support custom elements in the Parsoid DOM Spec, then there should be a discussion with the affected stakeholders.

Change #1143095 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/Chart@master] Use regular <div> instead of custom element <wiki-chart>

https://gerrit.wikimedia.org/r/1143095

The above patch demonstrates rendering the chart with a <div> and updating it with the wikipage.content hook, without adding any complexity.

The benefits of webcomponents compared to html tags is well documented. Webcomponents are html stanadard and supported by all browsers. Ofcourse, you can achieve the same features with html tags and javascript - with trade-offs. I would like to see them supported in mediawiki as they are part of web standard.

The abstraction also allows reuse of a frontend feature outside mediawiki. For example, <wiki-chart> can be used in any html page and we can get same rendering of our chart. This makes our charts reusable and embeddable in general web.

LPL team uses webcomponent for the page collection features. For example, https://meta.wikimedia.org/w/index.php?title=List_of_articles_every_Wikipedia_should_have/Expanded/Technology&action=edit - at the bottom webcomponent <page-collection> acts as a marker for the page and we are working on the rendering part of that.

Should we work on a decision brief to support webcomponents? They were added to products because it is a standard and it is functional with the current parser. If there are best practices to follow, we can very well document it so that VE can support it .

Would it be feasible to use a "customized built-in element" instead of a "custom element", e.g. <div is="wiki-chart">…</div> instead of <wiki-chart>…</wiki-chart>? That might be more compatible with other tools, and would only require a polyfill/workaround for Safari. https://developer.mozilla.org/en-US/docs/Web/API/Web_components/Using_custom_elements#types_of_custom_element

Would it be feasible to use a "customized built-in element" instead of a "custom element", e.g. <div is="wiki-chart">…</div> instead of <wiki-chart>…</wiki-chart>? That might be more compatible with other tools, and would only require a polyfill/workaround for Safari. https://developer.mozilla.org/en-US/docs/Web/API/Web_components/Using_custom_elements#types_of_custom_element

I think the new team maintaining Charts can consider this (circa August/September) but since we are winding down the efforts of the existing taskforce, I am not willing to make any changes to the markup while that is happening. Changing markup now we are in production would require at least 4 weeks given it would impact cached HTML:

  1. Replacing the markup (1 week)
  2. supporting the old and new markup for a period of 2 weeks
  3. Removing support for the new markup. (1 week)

The reason we used Webcomponents was multiple. Expanding on the use case around portability (T393306#10803165) which should be useful for apps adoption in future, it made the code far simpler as web components avoid the need to write rendering code and provide security benefits since wikitext cannot output this custom element.

Using the is attribute seems viable, but it is essential that Parsoid can support web components in some form. I can see reader's products making larger use of webcomponents in future so would encourage we explore how that should work.

Since I am going on extended leave I will update @Catrope on this situation so he can lead it while I'm out.

To briefly update: part of the issue here is how we sanitize content cut-and-pasted into VisualEditor. VE uses a somewhat restrictive sanitizer. Supporting custom components naively would require updating the sanitizer every time to include each custom tag name defined by an extension.

Some options include: (a) using a <div is...> like recommended above, (b) using a different property to distinguish extension content (T331655#10880651), (c) having an extensible list of "allowed" tags so that Charts can 'properly' add its custom tag to the VE sanitizer, instead of maintaining multiple lists of allowed tags. There are somewhat related questions here about (1) regardless of the wrapper, how content *inside* the wrapper is expected to be sanitized, and (2) the security context and purpose of sanitization (extension tags as a loophole, can it be exploited?).

These are architectural questions we hope to discuss and resolve.

CCiufo-WMF moved this task from Backlog to Up Next on the Charts board.
CCiufo-WMF subscribed.

@Catrope will take a look at the possible solutions and propose a path forward.

Change #1164223 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/Chart@master] [WIP] Use <div is="wiki-chart"> instead of <wiki-chart>

https://gerrit.wikimedia.org/r/1164223