Page MenuHomePhabricator

TemplateStyles's use of dynamic Parser::$extTemplateStylesCache should be rewritten/removed
Open, Needs TriagePublic

Description

There's no guarantee on identity on the specific parser objects used during article pass. With both async fragment rendering as well as the Parsoid/Parser migration (which may cause different parts of the page to be variously rendered using the new Parsoid or legacy parser), the actual Parser objects seen may be different for different instances of an extension tag on the same page.

When PHP 8.2 disallowed dynamic property creation, the following error was given (T324890):

Deprecated: Creation of dynamic property Parser::$extTemplateStylesCache is deprecated in /var/www/wiki/mediawiki/extensions/TemplateStyles/includes/Hooks.php on line 208

This was "fixed" by allowing dynamic properties on the Parser class, but this fix should eventually be reverted once the underlying usage in TemplateStyles is removed.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
cscott renamed this task from Cite's use of dynamic Parser::$extTemplateStylesCache should be rewritten/removed to TemplateStyles's use of dynamic Parser::$extTemplateStylesCache should be rewritten/removed.Aug 1 2023, 2:32 PM
cscott edited projects, added TemplateStyles; removed Cite.
cscott updated the task description. (Show Details)

See rETST99640093465f: Cache processed stylesheets during the parse. The TemplateStyles parsing process does not depend on parser options, so we could just cache in some global location instead; that makes it hard to say when the cache is not needed anymore, but I doubt that would cause any practical problems.

TemplateStyles parsing is not actually context-independent (as we want the first occurrence of a <templatestyles> reference to a given CSS page to turn into a <style> tag but any subsequent ones into a no-op reference to that styles tag) and Parsoid will have to accommodate that somehow, but this specific attribute is not related to that.

TemplateStyles parsing is not actually context-independent (as we want the first occurrence of a <templatestyles> reference to a given CSS page to turn into a <style> tag but any subsequent ones into a no-op reference to that styles tag) and Parsoid will have to accommodate that somehow, but this specific attribute is not related to that.

We handle this with a post processing step at the very end of the parse, when all the fragments have been assembled, which deduplicates the output. This is already present in ParserOutput::getText() I believe (option deduplicateStyles).

Oh right, sorry, I forgot we ended up implementing that outside the parser.

Change 1013134 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/extensions/TemplateStyles@master] Create TemplateStylesContentProvider service to hold and fill cache

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