Page MenuHomePhabricator

useParsoid needs to work with FlaggedRevs
Closed, ResolvedPublic

Description

During testing, discovered that a few wikis (plwki, arwiki) weren't respecting the useparsoid flag and returning legacy parser content when not logged in. While logged in, there was no problem. @Arlolra dug around and eventually hypotehsized that this might be FlaggRevs wikis and sure enough, dewiki, ruwiki also had the same problem.

I am not familiar with the FlaggedRevs extension, but I dug around the source and here is what I found so far:

Article::view in core calls the onArticleViewHeader(..) hook and if that returns a parser output, view() bails and uses the object it gets there. FlaggedRevs implements ths hook in frontend/FlaggedRevsUIHooks.php. This implementation calls setPageContent() in frontend/FlaggablePageView.php which "Replaces a page with the last stable version if possible " according to the doc in the header. Presumably logging in lets us view the unstable version which is why logging in works.

setPageContent() calls showStableVersion() which effectively calls FlaggedRevs::parseStableRevision() which parses the page there.

So, all the useparsoid() code in core is bypassed here.

So, we need to dig into this and make sure FlaggedRevs get ParsoidParser object when useparsoid=1

Event Timeline

ssastry added subscribers: cscott, daniel.

Following the breadcrumbs a bit more, the parsing is eventually delegated back to WikitextContentHandler::fillParserOutput which looks up parserOptions to decide whether to call Parsoid or legacy parser. However, the paresrOptions object comes from a new ParserOptions created in FlaggedRevs that eventually lands in WikiPage::makeParserOptions(..).

So, this may be the case of making sure all 'makeParserOptions' calls in core that create new parser options objects know to do the right thing when 'useparsoid=1' is passed in as a query param.

Aha .. so, ParserOptions::setUseParsoid() is called from the Parsoid extension in the Parsoid codebase via the onArticleParserOptions() hook that @cscott added in 2020 for discussion tools. But, it is only called in Article::view(). So, any newly created ParserOptions object will not get this hook called and hence the whole chain of events above.

The parser options object created in Article.php doesn't make its way to the FlaggedRevs extension's onArticleViewHeader hook implementation, and FlaggedRevs recreates it. I suppose, thus far, all the necessary state was captured in the user / context object and there wasn't a need to pass around ParserOptions. But, that assumption is not valid now. Short of calling the hook again (which we cannot), or passing in the original parser options object (which requires a change to the hook signature), this is going to be annoying to fix.

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

[mediawiki/extensions/FlaggedRevs@master] WIP: Support useparsoid query option

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

Test wiki created on Patch demo by SSastry (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/6c133be1d3/w

Test wiki created on Patch demo by SSastry (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/7576229393/w

Test wiki on Patch demo by SSastry (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/7576229393/w/

Test wiki on Patch demo by SSastry (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/6c133be1d3/w/

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

[mediawiki/core@master] ParsoidParser: Record ParserOptions watcher on ParserOutput object

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

Change 935828 merged by jenkins-bot:

[mediawiki/core@master] ParsoidParser: Record ParserOptions watcher on ParserOutput object

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

Change 934035 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Support useparsoid query option

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

Confirmed working in production now.