Page MenuHomePhabricator

Transclusion of pages with CSS/JS/JSON/LUA content model transcludes the plain text without styling
Open, Needs TriagePublic

Description

Hi. Open some page for source edit and add "{{some js or css or json or lua page}}". It looks terrible, especially json. I believe it should be fixed or at least prevented at all until the fix. Thank you.

For example:
pasted_file (950×1 px, 114 KB)
When transcluded:
pasted_file (950×1 px, 80 KB)

Event Timeline

matmarex renamed this task from Fix contentmodel page transclusion to Transclusion of pages with CSS/JS/JSON content model transcludes the plain text without styling.Sep 29 2016, 8:10 PM
matmarex subscribed.

The Content class has a getWikitextForTransclusion() method, possibly CssContent/JavaScriptContent/JsonContent should override it with… something.

Well, @matmarex, what do you think about (in pseudocode)

return $('<syntaxhighlight>', {lang: 'css', html: super()});

will it work?

Yes, basically, with two caveats:

  • SyntaxHighlight is an extension, so MediaWiki core would have to use <pre> and add a hook that SyntaxHighlight can use to change it to <syntaxhighlight>.
  • MediaWiki's XML-like tags are not nestable, so if the page being transcluded contained </pre> itself, the styling would stop in the middle of the page. This can probably be avoided with some clever parser abuse.
IKhitron renamed this task from Transclusion of pages with CSS/JS/JSON content model transcludes the plain text without styling to Transclusion of pages with CSS/JS/JSON/LUA content model transcludes the plain text without styling.Sep 30 2016, 12:00 PM
IKhitron updated the task description. (Show Details)

You are the boss, @matmarex. I also added the lua model.

Anomie subscribed.

Parsoid would probably also need something done here, if only replacing whatever it currently does to get the wikitext for a transclusion with something that calls getWikitextForTransclusion(). And in turn that might need some change to the action API.

Parsoid would probably also need something done here, if only replacing whatever it currently does to get the wikitext for a transclusion with something that calls getWikitextForTransclusion(). And in turn that might need some change to the action API.

Parsoid calls the expandtemplates action api endpoint, so as long as that does the right thing, this should work in Parsoid.

JJMC89 raised the priority of this task from High to Needs Triage.May 18 2018, 5:43 PM

Transclusion of pages with CSS/JS/JSON/LUA content model transcludes the plain text without styling

This description is not quite right. What actually happens is that MediaWiki treats the transcluded text as wikitext, so if there's any wikitext-like syntax in the transcluded page, it'll try to parse it. Otherwise, it does its best at trying to parse it as wikitext, and it's usually a case of garbage in, garbage out.

I say this because if a plain text page is transcluded, it also attempts to treat that as wikitext. Any plain text should also be wrapped in <pre> tags per T202424: Implementing line breaks on wiki viewer for plain text content model -- which would also address {T402019} (🤫 security).

Finally, it looks like the Vue content model is being introduced as well.

They can made to appear styled today using {{#tag:syntaxhighlight|{{MediaWiki:Common.css}}|lang=css}}.

Well, @matmarex, what do you think about (in pseudocode)

return $('<syntaxhighlight>', {lang: 'css', html: super()});

will it work?

To prevent wikitext in the gadget code from being interpreted, sometimes there are already syntaxhighlight tags, like for example at https://fr.wikipedia.org/wiki/MediaWiki:Gadget-PaStec.js. will nested syntaxhighlight do as expected ?

I'd propose just using wfEscapeWikiText() on the contents in getWikitextForTransclusion() so that the results are "safe" and won't get garbled by interpreting it as wikitext. "<pre>" . wfEscapeWikiText($contents) . "</pre>" is probably reasonable as well; the wfEscapeWikiText should ensure that any </pre> or other extension tags are entity-escaped and won't break the <pre>. (<pre> does do special handling of <nowiki> but I think the wikitext escape will handle that as well.)

I'd further suggest that this be the *default* implementation of getWikitextForTransclusion, rather than force all contentmodel extensions to have to "remember" to do this properly. WikitextContent would be the only case where getWikitextForTransclusion would return the content directly.

Change #1181190 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] Don't transclude non-wikitext content as wikitext

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

Change #1181201 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/ProofreadPage@master] Ensure that redirect content is returned as wikitext

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

Change #1181211 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Scribunto@master] Don't use Content::getWikitextForTransclusion to fetch Scribunto sources

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

Change #1181212 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] Return RevisionRecord from Parser::fetchTemplateAndTitle()

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

I'm not totally sure what wfEscapeWikitext actually does (the documentation is unclear), but note that there may be wikilinks, or URLs inside the CSS/Javascript pages (etc, but not plain text pages) that are rendered normally in CSS/Javascript pages (though not plain text pages).

I think how the CSS appears on a CSS content model page should be the same as how it appears when CSS is transcluded into a wikitext page, links and all. The same for any other type.

@Bugreporter2 This should not affect how CSS/Json appears *on the actual page*, just how it appears when transcluded from wikitext. In a default mediawiki install, CSS/JSON is displayed as a <pre> block without highlighting or links. That styling is added by the SyntaxHighlight extension. I can consider adding a hook to allow the SyntaxHighlight extension transform the default rendering of CSS/JSON transcluded into a wikitext page into <syntaxhighlight>...contents...</syntaxhighlight> instead of <pre>...</pre> contents, but that's going to be a hook on top of the base mediawiki functionality.

Change #1182194 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Cite@master] MediaWiki messages with non-wikitext types may be escaped

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

@Bugreporter2 This should not affect how CSS/Json appears *on the actual page*, just how it appears when transcluded from wikitext. In a default mediawiki install, CSS/JSON is displayed as a <pre> block without highlighting or links. That styling is added by the SyntaxHighlight extension. I can consider adding a hook to allow the SyntaxHighlight extension transform the default rendering of CSS/JSON transcluded into a wikitext page into <syntaxhighlight>...contents...</syntaxhighlight> instead of <pre>...</pre> contents, but that's going to be a hook on top of the base mediawiki functionality.

OK, but what about the embedded wikilinks and URLs?

OK, but what about the embedded wikilinks and URLs?

There are no wikilinks or URLs embedded by the parser or content handler. They are merely added via JS in SyntaxHighlight. The links will work if syntaxhighlight tags are used instead of pre.

OK, but what about the embedded wikilinks and URLs?

There are no wikilinks or URLs embedded by the parser or content handler. They are merely added via JS in SyntaxHighlight. The links will work if syntaxhighlight tags are used instead of pre.

OK that makes sense.

Change #1182958 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/ProofreadPage@master] MediaWiki messages with non-wikitext types may be escaped

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

Change #1181201 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Ensure that redirect content is returned as wikitext

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

Change #1182958 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] MediaWiki messages with non-wikitext types may be escaped

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

@cscott I'm not sure that the switch is actually needed. I don't think third party wikis will use it like that, but if they are, they're using MediaWiki wrong. And as a security issue, I'm not sure that allowing people to bypass it is a good idea, especially considering that we need to assume that most third party users aren't super tech-savvy.

I think what's actually needed is to check that the behaviour of the following doesn't really change due to the insertion of <pre> tags:

<syntax highlight lang=CSS>{{#some transcluded CSS page}}</syntaxhighlight>