Page MenuHomePhabricator

TemplateStyles puts css in page_props table, which might be subject to race conditions, etc
Closed, ResolvedPublic

Description

TemplateStyles uses the page_props table to store its css. However, that table is updated independently of saving a page. It might also not have the right version, depending on parser altering user preferences. I think it would be better to just directly use $parser->getOutput()->addHeadItem(). This ensure's that the user always see's the version of the css that corresponds to the version of the page currently viewing, and if LinksUpdate is delayed (due to job queue issues), you don't get weird inconsistencies, or bad versions cached due to race conditions.

Related Objects

StatusAssignedTask
OpenNone
OpenNone
ResolvedNone
DuplicateNone
OpenNone
OpenNone
OpenNone
DuplicateNone
OpenNone
OpenNone
ResolvedJdlrobson
DuplicateNone
DuplicateNone
OpenNone
OpenNone
StalledNone
InvalidNone
OpenNone
OpenNone
ResolvedTheDJ
ResolvedTheDJ
InvalidNone
OpenNone
ResolvedTheDJ
OpenNone
ResolvedJdlrobson
OpenJdlrobson
OpenJdlrobson
ResolvedTgr
ResolvedAnomie

Event Timeline

Bawolff created this task.May 24 2016, 12:08 AM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptMay 24 2016, 12:08 AM
Krinkle renamed this task from Template styles stores the css in page_props table, which might be subject to race conditions, etc to TemplateStyles puts css in page_props table, which might be subject to race conditions, etc.Jul 1 2016, 7:59 PM
Krinkle added a subscriber: Krinkle.

I think it would be better to just directly use $parser->getOutput()->addHeadItem(). This ensure's that the user always see's the version of the css that corresponds to the version of the page currently viewing

It should use the version of the css that corresponds to the version of the page being used in the parser output (especially with regards to transclusions). Can we guarantee that somehow? So that whatever version is used as part of template expansion, is kept and then used for the templatestyles as well.

I think it would be better to just directly use $parser->getOutput()->addHeadItem(). This ensure's that the user always see's the version of the css that corresponds to the version of the page currently viewing

It should use the version of the css that corresponds to the version of the page being used in the parser output (especially with regards to transclusions). Can we guarantee that somehow? So that whatever version is used as part of template expansion, is kept and then used for the templatestyles as well.

That is what using properties of the ParserOutput would do. (Provided that the extension abandoned the idea of putting the tag inside a <noinclude>).

Tgr added a subscriber: Tgr.Dec 10 2016, 12:41 AM

On FlaggedRevs wikis the CSS from the stable version of the template needs to be shown on stable articles, which complicates the page_props approach even further. And then there is the issue of displaying old revisions of the template.

Change 346808 had a related patch set uploaded (by Anomie):
[mediawiki/extensions/TemplateStyles@master] Use wikimedia/css-sanitizer, and rewrite the hooking

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

Change 346808 merged by jenkins-bot:
[mediawiki/extensions/TemplateStyles@master] Use wikimedia/css-sanitizer, and rewrite the hooking

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

Anomie closed this task as Resolved.Jun 7 2017, 4:13 PM
Anomie claimed this task.
ggellerman moved this task from Up next to Done on the TemplateStyles board.Nov 21 2017, 7:15 PM