Page MenuHomePhabricator

Feature flag ("kill switch") for TemplateStyles
Closed, ResolvedPublic

Description

It is extremely unlikely that deploying TemplateStyles could result in any issue that would want us to temporarily disable it, but just in case, we should make disabling it easy. Simply turning off the extension would litter the code with visible <templatestyles> tags, so there should probably be a config flag to make it still recognize the tag but not act on it in any way. (Or maybe just a prepared config patch for overriding the parser callback, if that's easy to do.)

Details

Related Gerrit Patches:
mediawiki/extensions/TemplateStyles : masterTemporary feature flag for disabling style output

Related Objects

StatusAssignedTask
OpenNone
OpenNone
ResolvedNone
DuplicateNone
OpenNone
OpenNone
OpenNone
DuplicateNone
OpenNone
OpenNone
Resolved Jdlrobson
DuplicateNone
DuplicateNone
OpenNone
OpenNone
DeclinedNone
InvalidNone
OpenNone
OpenNone
ResolvedTheDJ
ResolvedTheDJ
InvalidNone
OpenNone
ResolvedTheDJ
OpenNone
Resolved Jdlrobson
Open Jdlrobson
Open Jdlrobson
ResolvedTgr
ResolvedTgr

Event Timeline

Tgr created this task.Sep 19 2017, 9:16 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 19 2017, 9:16 PM
Anomie closed this task as Declined.Sep 19 2017, 10:24 PM
Anomie added a subscriber: Anomie.

I think YAGNI applies here. Your feature flag should be easy enough to add if it's actually needed, as all that would really be needed to do what you propose would be to add a return ''; at the top of TemplateStylesHooks::handleTag(), and maybe something similar for TemplateStylesHooks::onParserAfterTidy() depending on what is inexplicably breaking.

dr0ptp4kt reopened this task as Open.Sep 26 2017, 9:01 PM
dr0ptp4kt added a subscriber: dr0ptp4kt.

Re-opening, Gergo to implement - this is based on product manager request as a risk mitigation. None of us believe this is particularly hard (although there are probably differences of opinion about YAGNI), although having a configuration variable might at least streamline things in case one wiki is having an issue but all the others are okay.

Legoktm added a subscriber: Legoktm.Oct 3 2017, 6:26 PM

I think it would be helpful to explain what kinds of issues you think might be encountered that would require the use of this? Exceptions on every page view? Some exploit in the CSS sanitizer that allows for XSS? In either case I wouldn't really be worried about raw <templatestyles> tags in output, I'd be more concerned with clearing the parser cache and purging pages.

@dr0ptp4kt thanks for indicating that @Tgr will work on this.

@Deskana Product Owner and I are prioritizing backlog. Assigning to @Tgr. Thanks, Gergo!

ggellerman assigned this task to Tgr.Nov 2 2017, 5:46 PM
ggellerman triaged this task as Medium priority.

Change 405235 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/TemplateStyles@master] Temporary feature flag for disabling style output

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

Anomie moved this task from Up next to Doing on the TemplateStyles board.Jan 29 2018, 4:15 AM

Change 405235 merged by jenkins-bot:
[mediawiki/extensions/TemplateStyles@master] Temporary feature flag for disabling style output

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

Tgr closed this task as Resolved.Jan 29 2018, 9:08 PM

Closing as resolved. We still need to revert the patch once TemplateStyles is deployed everywhere and seems stable, but I don't think there's any need to track that in Phabricator.

ggellerman moved this task from Doing to Done on the TemplateStyles board.Feb 6 2018, 7:05 PM