This class was added to resolve T37247: content-holding <div> should only contain the page text to fix pending issues with TemplateStyles. For the styles added by TemplateStyles to be applied properly in VE, VE's content editing area should have class mw-parser-output or be enclosed in an element with that class.
Chances are you can simply add the new class to the div that you're already adding mw-content-ltr or mw-content-rtl to.
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Add mw-parser-output class to CE document | mediawiki/extensions/VisualEditor | master | +3 -0 |
We don't set the mw-content-ltr class, we're a client – that's set by Parsoid. I think you've got the architecture mixed up, it'd be totally wrong for VE (and Flow and Mobile Content Service and Kiwix and…) to set classes about what is and isn't the Parsoid output. Is there a ticket for that?
Not that I know of, feel free to create a subtask if that's the appropriate way for it to happen.
And thanks for the quick reply.
Well VE is not parser output, but looking at the parent task I think that just means it is badly named. If that class is going to be used basically how mw-body-content is intended, then we will need to add mw-parser-output to VE surfaces, which will be very confusing.
Parsoid in Beta Labs is now outputting the mw-parser-output class, but it seems to be getting lost somewhere between there and VE.
For example, https://en.wikipedia.beta.wmflabs.org/api/rest_v1/page/html/Test_for_T164790?redirect=false includes <body id="mwAA" lang="en" class="mw-content-ltr sitedir-ltr ltr mw-body-content parsoid-body mediawiki mw-parser-output" dir="ltr">, and I see it in the output from https://en.wikipedia.beta.wmflabs.org/wiki/Special:ApiSandbox#action=visualeditor&format=json&page=Test+for+T164790&paction=parse too, but when actually editing the page in VE the class is not present on VE's editing surface.
Yeah, the VE editing surface is a complete reconstruction of the Parsoid content DOM.
@Jdforrester-WMF TemplateStyles is coming to the beta cluster shortly. Would it be possible to get a patch into VE along the lines in the Description shortly following it's introduction there? (I see the change in Parsoid didn't automagically do the trick)
I can't quite tell whether this task should block rollout to production, but I believe it's probably the sensible thing, in order to allow the VE-using editor to, for example, resize the viewport and see how editor-provided template styles would look ahead of a save.
@Esanders Do you have any recommendations on this? The deployment to beta is upcoming.
The main problem with this ticket as I understand it is that it implies an architecture that's entirely inaccurate. The HTML displayed in VE's editing area explicitly isn't parser output (either of the legacy parser or Parsoid), but VE's bespoke CE HTML that lets users interact with the content, and often has to work around a lot of the bugs in browsers. It's intentionally quite unlike Parsoid HTML in some areas.
For probably the most extreme example, <!--Foo--> becomes (simplified) <span class="ve-ce-commentNode …" contenteditable="false"><span class="…" aria-disabled="false"><a role="button" …><span class="oo-ui-labelElement-label">Foo</span></a></span></span>, which means it displays with an icon and you can interact with it.
Labelling two sets of HTML that are trying to be the same with the same class (the legacy parser and Parsoid) seems fine, but labelling the third when it's fundamentally different worries me a bit. HTML designed to work in one way might break in others. Selectors won't apply (or will wrongly), etc..
Maybe instead we should label rendered objects fetched from the parser (like transclusions) with this class and leave it at that? Not sure what would break, though.
Maybe instead we should label rendered objects fetched from the parser (like transclusions) with this class and leave it at that? Not sure what would break, though.
@Jdforrester-WMF what would the rough UX be like in the active ContentEditable under this option? Also, what's the nature of potential breakage you're thinking of here?
No change for VE users, except when TemplateStyles were in effect they'd get styled by that code.
Also, what's the nature of potential breakage you're thinking of here?
TemplateStyles would apply to the wrong things, or not apply when they should. Potentially it could be a little confusing to users and make people dissatisfied with the outcome.
I think that would work for the main intended use of TemplateStyles. Consider a template that outputs <span class="foo">foo!</span> with user-writen CSS like .foo { color: red; }. TemplateStyles will turn that CSS into .mw-parser-output .foo { color:red; }. If VE turns the HTML into something resembling <tag class="mw-parser-output"><span class="foo">foo!</span></tag> we should be good. But <span class="mw-parser-output foo">foo!</span> won't work, of course.
The potential problems there would be
Adding mw-parser-output to the CE surface is a trivial one line fix. If this class exists solely to support TemplateStyles then it's fine, otherwise it should be noted somewhere that this class does not guarantee the contents is parser output as the name suggests, just that it wants to be rendered as if it were.
Change 365651 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/VisualEditor@master] Add mw-parser-output class to CE document
User CSS affecting VE structures is already the status quo; currently such CSS is unprefixed and affects everything on the page, with TemplateStyles it will be sandboxed into the content area by prefixing .mw-parser-output to all CSS selectors. That's no change as far as the interaction between user styles and VE scaffolding is concerned. If mw-parser-output is misleading, maybe we could rename it to something like mw-user-generated-content?
I think the likelihood of accidents is fairly small as all sane CSS selectors contain something highly specific (like a class name or id). If someone makes a selector like span span a, that will interact with VE in unexpected ways, but it's probably impossible to predict its interaction with normal content as well, so it's unlikely anyone would do that.
As for using the class on each separate transclusion, how would that work after we switch to Parsoid for pageviews and VE does not replace the content anymore when the user presses edit? It seems like templates would break and only get fixed when the editor changes them in a way that triggers a reparse.
There'd be no change. The CE HTML would still be distinct from the read HTML, it's just that the editable data model would be created directly from the read HTML rather than fetched separately.
It seems like templates would break and only get fixed when the editor changes them in a way that triggers a reparse.
Any template styles code that tries to affect content outside of the template will indeed not work. I'd consider that a feature, not a bug. I don't think it'd be affected at all in the visual editing experience by Parsoid HTML being used for reads.
Change 365651 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Add mw-parser-output class to CE document
Any template styles code that tries to affect content outside of the template will indeed not work. I'd consider that a feature, not a bug.
FWIW there is one pretty solid use case for it: removing repetitive style markup from non-transcluded templates (column styles, zebra tables etc). Maybe in the future that can be handled by wrapping the table in a template, once there proposed heredoc syntax for templates gets implemented, but for now there is no way to do it in a scoped manner, and the usability gains from getting ridding article wikitext from all the row/cell style markup would be significant.