Page MenuHomePhabricator

Provide post-cache ParserOutput transformations
Closed, ResolvedPublic

Description

There are at least two things currently part of the ParserCache key, and thus part of the ParserOutput object state, that are not actually relevant for the Parser and are only used by ParserOutput::getText() at run-time:

  • Enable TOC.
  • Enable section edit links.

I'm proposing to deprecate these state properties and methods, and instead implement these as run-time parameters to ParserOutput::getText().

In addition, per T167784#3475991, it might be interesting to try and move the wrapclass option from the parser into ParserOutput. We may need to special-case wrapclass a bit if getRawText is meant to have the wrapper, but that still do-able.

Event Timeline

Krinkle created this task.Jul 26 2017, 10:37 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 26 2017, 10:37 PM

Change 368107 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] [WIP] ParserOutput: Implement getText() post-cache transform options

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

Krinkle updated the task description. (Show Details)Jul 26 2017, 10:41 PM

I'd say getRawText() shouldn't have transformations. It was introduced specifically to handle the case where the text is going to be injected back into a ParserOutput via setText() and transformations caused bugs like T124356: Incorrect TOC and section edit links rendering in Vector due to ParserCache corruption via ParserOutput::setText( ParserOutput::getText() ).

There are three cases with the wrapclass parser option:

  1. Wrapping with the default "mw-parser-output"
  2. No wrapping at all
  3. Wrapping with some non-default class

As far as actual parser output beyond the wrapper div itself, the only thing currently varying on this option is TemplateStyles. And TemplateStyles generates the same output for #1 and #2. So what we could probably do is:

  • Deprecate #2 (the ability to set wrapclass to false for "no wrapping") in favor of an "unwrap" transformation that removes the default wrapper div.
  • Change MessageCache/Message, TextExtracts, ParsoidBatchAPI, and MobileFrontend to use the "unwrap" transformation instead of setting false. Also update tests in TemplateStyles and Wikibase to do whatever is appropriate where they're currently setting false.
  • Remove wrapclass from ParserOptions::$inCacheKey, causing ParserOptions with a non-default wrapclass to become non-cacheable.

Change 393260 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] ParserOutput: Add stateless transforms to getText()

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

Change 393260 merged by jenkins-bot:
[mediawiki/core@master] ParserOutput: Add stateless transforms to getText()

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

Change 368107 abandoned by Krinkle:
[WIP] ParserOutput: Implement getText() post-cache transform options

Reason:
This is now done with Ied5fe1a6159c2d. Thanks!

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

Anomie closed this task as Resolved.Dec 1 2017, 7:48 PM
Anomie claimed this task.

The main work is done here, all that's left is merging the warnings and the hard deprecation. So let's mark this as Resolved.

The wrapclass bit is still a good idea. That's now T181846.