Page MenuHomePhabricator

Create ParsoidExperimentalDeprecatedPostProcessingHookDoNotUse hook for DiscussionTools testing
Closed, DeclinedPublic

Description

DiscussionTools uses the ParserAfterTidy hook, which is problematic in the Parsoid read-views context: because this applies pre-cache, it will affect the parsoid output used for VisualEditor/Flow/ContentTranslation and everyone else. So ideally DiscussionTools should apply this transformation *post*cache, but (a) that has performance implications, and (b) the ParserAfterTidy hook as written supplies a Parser $parser argument, and the parser object isn't available post-cache.

We have a longer-term plan to address this (https://www.mediawiki.org/wiki/Parsoid/ParserOutputTransform) but in order to unblock testing and discover more quickly what other unknown-unknowns might block the DiscussionTools-Parsoid-Read-Views work, we propose to introduce a new ParsoidExperimentalDeprecatedPostProcessingHookDoNotUse that Parsoid will apply post-cache (and which does not supply a $parser argument), and tweak DiscussionTools (temporarily!) to implement this hook. That will let us continue our visual diff and other testing in the short term while we build out the ParserOutputTransform framework in core.

Event Timeline

cscott updated the task description. (Show Details)

The full proposal is at https://www.mediawiki.org/wiki/Parsoid/OutputTransform, "Add a hook in FlavorDispatcher to allow inserting additional passes into the chain for any flavor; this will replace the ParserOutputAfterTidy hook."

Note that DT really cares a lot that the post-DT output is cached; see discussion in https://www.mediawiki.org/w/index.php?title=Parsoid%2FOutputTransform&diff=6086524&oldid=6051987 -- the experimental hook will probably not be cached, and that will have to be resolved before read views deployment.

MSantos triaged this task as High priority.Sep 19 2023, 3:42 PM

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

[mediawiki/core@master] ParsoidParser: Record page title in ParserCache entries

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

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

[mediawiki/core@master] WIP: Parsoid: Add a html pre-render (post-cache) transform hook

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

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

[mediawiki/extensions/DiscussionTools@master] WIP: Add ParserOutputPostTransformHook handler for Parsoid HTML

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

Change 969227 abandoned by Subramanya Sastry:

[mediawiki/core@master] WIP: Add a Parsoid HTML pre-render (post-cache) transform hook

Reason:

Looks like we aren't going down this path.

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

ssastry moved this task from Code Review to To Deploy on the Content-Transform-Team-WIP board.

We ended up reusing an existing hook and not creating a new hook for this purpose .The abandoned code is all in gerrit if we ever need a new hook again. So, declining this.