Page MenuHomePhabricator
Create Task
Maniphest T136054

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

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.

Details

SubjectRepoBranchLines +/-
Use wikimedia/css-sanitizer, and rewrite the hookingmediawiki/extensions/TemplateStylesmaster+1 K -953
Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 DeclinedNoneT208761 [Epic] Improve PHPUnit coverage in MobileFrontend to 50%
 ResolvedJdlrobsonT186627 Remove the MobileView API
 DeclinedNoneT173379 Remove mobileview API call for main page
 DuplicateNoneT172525 [EPIC] Deprecate MobileView API in MobileFrontend
 ResolvedJdlrobsonT32405 [EPIC] MobileFrontend extension should stop special-casing main page
 DuplicateNoneT138449 Throw warning when edits made to a main page which uses special casing
 DuplicateNoneT138622 Help community migrate away from legacy Main page special casing
 DeclinedNoneT125920 [EPIC] Future exciting reading web performance endeavours
 ResolvedJdlrobsonT131423 FOUC in section "Recently featured" on mobile Main Page
 DuplicateNoneT126137 FOUC for styles applied via MediaWiki:Mobile.css due to bottom-loading
 DuplicateNoneT130846 Hat-note FOUC on mobile skin
 OpenbwangT361737 [EPIC] With the current layout, some elements extend beyond the content area and overlap sticky page tools
 DuplicateovasilevaT66577 Large tables created by editors are not responsive
 ResolvedNoneT51440 MobileFrontend images shown double-sized in Retina desktop, iOS 7 beta
 OpenNoneT40848 Security: CSS positioning can be used to break out of the content area
 OpenNoneT37704 Drop support in wikitext for inline styles
 ResolvedTheDJT151170 Varying layouts for commons mobile main page
 DeclinedNoneT125257 Infoboxes should take the max. available width of the screen
 InvalidNoneT135857 Map points aligned to viewport on mobile
 OpenFeatureNoneT17075 Per book, category and/or template CSS and JavaScript
 InvalidNoneT109152 [EPIC] All templates on wiki should work in mobile
 ResolvedTheDJT94107 {{hat}} and similar templates on Wikipedia are invisible to mobile web view.
 ResolvedTheDJT118062 In the mobile frontend, in the list produced by Template:Notelist-ua, efn-ua footnotes appear with numeric labels instead of uppercase-alpha. They appear normal in the text, and in the desktop frontend.
 InvalidNoneT120238 Wrong display of certain English Wikipedia portal pages in mobile
 ResolvedIznoT127727 navigational sidebar on mobile, should float on wide screens.
 ResolvedTheDJT128095 Signpost is not readable on mobile devices
 OpenNoneT117950 Remove the need for `.noprint`
 ResolvedJdlrobsonT117951 Remove templaters' need for .nomobile and support authoring media queries for templates
 DeclinedNoneT66578 Migrate css rules in hacks.less into templates
 ResolvedTgrT133410 Deploy TemplateStyles to WMF production
 ResolvedAnomieT136054 TemplateStyles puts css in page_props table, which might be subject to race conditions, etc

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
Bawolff mentioned this in T133408: Security review of TemplateStyles.Jun 7 2016, 7:59 PM
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 subscribed.

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.

Bawolff added a comment.Jul 1 2016, 8:05 PM
In T136054#2422577, @Krinkle wrote:

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 subscribed.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.

Tgr mentioned this in T135789: Wrapping templatestyle tag in a noinclude was not intuitive..Dec 10 2016, 12:41 AM
Tgr added a parent task: T133410: Deploy TemplateStyles to WMF production.
gerritbot subscribed.Apr 6 2017, 6:48 PM

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

gerritbot added a project: Patch-For-Review.Apr 6 2017, 6:48 PM
gerritbot added a comment.Jun 7 2017, 3:37 PM

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 Backlog to Done on the TemplateStyles board.Nov 21 2017, 7:15 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits