Page MenuHomePhabricator

Parsoid-native implementation for <indicator> (page status indicators)
Closed, ResolvedPublic

Description

TL:DR; Page status indicators ( https://www.mediawiki.org/wiki/Help:Page_status_indicators ) require native Parsoid support since the body can have arbitrary wikitext (like <ref>) which should be handled natively. Chat transcript with Bartosz below that explains this.

<subbu> so, are page status indicators an extension?
<MatmaRex> no, it's a core feature of MediaWiki
<MatmaRex> but they're implemented using "extension tag", or "tag hook", like e.g. <gallery>
<subbu> right .. so, i am trying to figure out if parsoid needs custom support for it.
<subbu> or, if will show up as a registered extension and we'll get that handled via the normal extension parse req. to the m/w api.
<subbu> i guess i could try it out. :)
<MatmaRex> seems that it does should up, i didn't even realize, huh.
<MatmaRex> you might want to do something special to treat it as a meta item, though.
<subbu> but, it is not generating any output right now.
<subbu> echo '<indicator name="foo">[[File:Foo.svg|20px]]</indicator>' | node parse
<subbu> <span data-parsoid='{"dsr":[0,55,2,2]}' typeof="mw:Extension/indicator" data-mw='{"name":"indicator","attrs":{"name":"foo"},"body":{"extsrc":"[[File:Foo.svg|20px]]"}}' about="#mwt3">
<subbu> </span>
<subbu> will have to investigate if that is a parsoid bug, or if that is the right output for that indicator
<subbu> do they need special editing support in VE?
<MatmaRex> subbu: i am not as up to date on how VE works as i'd like to be, but as far as i know:
<subbu> offhand, do you have an example page with indicators that I can use to play with this?
<MatmaRex> subbu: indicators should be treated as meta items (like [[category:foo]], etc.), but their contents should be parsed like "subdocuments" (like <ref>, i guess?) and VE should be given the parsed HTML (and the ability to send it back). note that they will likely be usually template-generated
<MatmaRex> let me find one
<MatmaRex> subbu: on this page: https://pl.wikipedia.org/wiki/Zręczyce the coordinates in top-right are generated with <indicator>, somewhere deep in the infobox. (let me find something simpler :) )
<MatmaRex> subbu: simpler example: https://pl.wikipedia.org/wiki/Kwatera_na_Łączce the <indicator> comes from the {{koordynaty|52.255736|N|20.952429|E}} call at the top of the article
<subbu> so, http://parsoid-lb.eqiad.wikimedia.org/plwiki/Zr%C4%99czyce?oldid=42190879 doesn't generate anything. so, looks like we'll need some patches to core to pass back indicator information as part of the api call.
<subbu> otherwise, when they are templated, parsoid will never know about their presence.
<subbu> just like categories, and other resources (css, js).
<MatmaRex> subbu: https://www.mediawiki.org/wiki/Phabricator also has an indicator in top-right
<MatmaRex> subbu: you're using api=expandtempltaes, right?
<subbu> api=parse for this
<MatmaRex> or parse?
<MatmaRex> you can pass api=parse&indicators=1, i think
<subbu> oh, i see. 
<MatmaRex> to get… something out. i don't quite remember :P
<subbu> ok .. thanks. 
<subbu> MatmaRex, &prop=indicators does it.
<subbu> ex: https://en.wikipedia.org/w/api.php?action=parse&format=json&text=%3Cindicator%20name=%22foo%22%3E[[File:Foo.svg|20px]]%3C/indicator%3E&prop=text|modules|jsconfigvars|categories|indicators
<subbu> MatmaRex, so, can indicator body have arbitrary wikitext?
<MatmaRex> subbu: technically yes
<subbu> hmm .. 
<MatmaRex> (although obviously it will start looking very silly pretty fast)
<subbu> so, that is an issue.
<MatmaRex> it probably doesn't need to be arbitrary, there's no reason to put anything block-level there, but it's not easy to limit that in the wikitext parser
<subbu> so, for indicators that are templated, since parsoid doesn't see the actual wikitext, we'll end up mixing php-parser-generated html for wikitext with parsoid-generated html.
<MatmaRex> (we still want links, images, text formatting)
<subbu> over time, they will converge, but, in the interim, editability of indicators will suffer.
<subbu> for top-level indicators, parsoid can implement a native extension for it and not ask php parser for the output.
<subbu> and they can be edited. so, maybe not an issue after all.
<subbu> since only top-level indicators are directly editable .. and templated content can never be.
<MatmaRex> yeah. i think that works out well enough?
<subbu> alright, I think so, we are looking at a native parsoid impl. for this. and rely on the api=parse output for templated indicators.
<MatmaRex> yeah. toplevel indicators are probably rare, i doubt you'll encounter them in the wild, apart from user pages maybe

Related Objects

StatusSubtypeAssignedTask
OpenReleaseNone
OpenNone
OpenNone
OpenNone
OpenFeatureNone
OpenNone
OpenNone
OpenNone
Resolvedssastry
StalledNone
Resolvedssastry
Resolvedcscott
Resolvedcscott
Resolvedcscott
Resolvedcscott
ResolvedPRODUCTION ERRORLucas_Werkmeister_WMDE
ResolvedPRODUCTION ERRORabi_
ResolvedPRODUCTION ERRORcscott
ResolvedPRODUCTION ERRORroman-stolar
ResolvedNone
ResolvedNone
Resolvedmatmarex
Resolvedmatmarex

Event Timeline

ssastry raised the priority of this task from to Needs Triage.
ssastry updated the task description. (Show Details)
ssastry subscribed.
ssastry triaged this task as Medium priority.Jul 29 2015, 7:15 PM
ssastry updated the task description. (Show Details)
ssastry set Security to None.
ssastry added a project: Parsoid.

Within Parsoid's native implementation, it is possible to enforce additional constraints on the output of the indicator tag, i.e. remove block tags, etc or sanitize in other ways.

I was not thinking straight in the conversation above. If indicators are templated, Parsoid gets the expanded wikitext with the indicator extension tags. So, while we can get the PHP parser expansion by relying on action=parse temporarily, the right solution is to implement this extension tag natively in Parsoid. All it needs to do is parse the wikitext body and add it to a div (with the right classes) outside the main content. It will pick up the right styles based on the class names.

while we can get the PHP parser expansion by relying on action=parse temporarily

Since this is usually displayed outside the main content, the return from action=parse will be empty (as your results showed above)

https://github.com/wikimedia/mediawiki/blob/master/includes/parser/CoreTagHooks.php#L174

All it needs to do is parse the wikitext body and add it to a div

There's a bit of a fundamental limitation here, $parser->recursiveTagParseFully( $content, $frame )

https://github.com/wikimedia/mediawiki/blob/master/includes/parser/CoreTagHooks.php#L171

The return from our call to action=expandtemplates will lose frame information. This applies more generally than just to indicator. Any extension tags that parse wikitext that are nested in templates and want to resolved template args at the current frame depth won't work.

Any extension tags that parse wikitext that are nested in templates and want to resolved template args at the current frame depth won't work.

Similarly, the <infobox> tag's call to PPFrame::getArguments()

LGoto raised the priority of this task from Medium to High.Jun 25 2020, 6:18 PM
LGoto added a project: Parsing-Active-Work.
ssastry lowered the priority of this task from High to Medium.Sep 24 2020, 6:26 PM

Indicators are part of the ParserOutput, so this ends up falling into T287216. Where does the actual <indicator> extension tag implementation live, though? I don't immediately see it in https://codesearch.wmcloud.org/search/?q=setIndicators&i=nope&files=&excludeFiles=&repos=

See T300980: Store page indicators as a set; select one at output time, in particular T300980#7704808 has a recommendation for an improved API in ContentMetadataCollector.

Change 901331 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/services/parsoid@master] WIP: Indicators support

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

Change 931325 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/services/parsoid@master] WIP: Add support for indicators

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

Change 901331 abandoned by Subramanya Sastry:

[mediawiki/services/parsoid@master] Add support for indicators

Reason:

In favor of https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/931325

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

Change 931325 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Add support for indicators

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

Change 938893 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.18.0-a17

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

Change 938893 merged by jenkins-bot:

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.18.0-a17

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

This is done now. I'll file a follow up task to check what we should do, if anything, about the fact that Parsoid generates inline data-parsoid & data-mw right now for indicator HTML. Check output of any page, for ex, https://en.wikipedia.org/wiki/Hampi_(town)?useparsoid=1 .. and you'll see inline data-parsoid in there.