Page MenuHomePhabricator

Deprecate the SkinTemplateOutputPageBeforeExec hook
Closed, ResolvedPublic2 Estimated Story Points

Description

NOTE: For developers needing to update code using the SkinTemplateOutputPageBeforeExec hook please see subtasks and this guide

This hook is very generic and very dangerous adding a lot of risk to support for legacy and modern Vector - it allows any extension to redefine template variables in a skin and as a result change the behaviour of a skin. In the past we've run into various problems in the skin in MobileFrontend where language_urls in the context of our skin means a list of languages but that seems to differ from core.

I believe we should understand the existing uses of it and provide more generic hooks to guide usage.

e.g. in the case of languages a SkinAddLanguageUrl hook would be better as this would also change the return result of getLanguages.

https://codesearch.wmflabs.org/search/?q=SkinTemplateOutputPageBeforeExec&i=nope&files=&repos=

Acceptance criteria

https://www.mediawiki.org/wiki/Category:SkinTemplateOutputPageBeforeExec_extensions

Current users of SkinTemplateOutputPageBeforeExec hook per https://codesearch.wmflabs.org/ (as of 2018-01-30)

  • BreadCrumbs2
    • $tpl->set( 'subtitle', .. )
    • $template->data['sidebar'][ .. ] = ..
    • $wgLogo = ..
  • CookieWarning
    • $tpl->data['headelement'] .= ..
  • ExtraLanguageLink
    • $sk->getOutput()->getProperty( 'extralanguagelinks' )
    • $tpl->get( 'language_urls' )
    • $tpl->set( 'language_urls', .. )
  • StickToThatLanguage
    • $tpl->set( 'language_urls',.. )
  • Wikibase
    • $template->set( 'language_urls', [] )
    • $template->set( 'wbeditlanglinks, .. )
  • InterlanguageExtension
    • $template->data['language_urls'][] = ..
  • BlueSpiceFoundation, BlueSpiceReaders, BlueSpiceAuthors
    • $template->data['bs_..'][] = ..
  • HitCounters
    • $tpl->get( 'footerlinks' )
    • $tpl->set( 'footerlinks', .. ), $tpl->set( 'viewcount', .. )
  • MobileFrontend
    • $tpl->data['footerlinks']
    • $tpl->set( 'mobileview', .. )
    • $tpl->set( 'footerlinks', .. )
    • $tpl->set( 'desktop-toggle', .. ), $tpl->set( 'mobile-license', .. ), $tpl->set( 'privacy', .. ),$tpl->set( 'terms-use', .. )
  • SharedHelpPages
    • $tpl->set( 'copyright', .. )
  • UniversalLanguageSelector
    • $template->get( 'language_urls' ), $template->set( 'language_urls', .. )
  • WhoIsWatching
    • $tpl->set( 'numberofwatchingusers', .. )
  • Wikidata.org
    • $template->data['footerlinks'][] = ..,$template->set( .. )
  • WikidataPageBanner
    • $tpl->set( 'prebodyhtml', .. ), $tpl->get( 'prebodyhtml' )
  • WikimediaMessages
    • $template->data['footerlinks'][] = ..,$template->set( .. )

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Krinkle renamed this task from Kill SkinTemplateOutputPageBeforeExec hook to Remove SkinTemplateOutputPageBeforeExec hook.Jan 10 2019, 7:05 PM

Here I am 7 years later and hitting this again. Now is the time to "move towards narrower and more precisely defined interfaces. " :)

Jdlrobson renamed this task from Remove SkinTemplateOutputPageBeforeExec hook to [EPIC] Remove SkinTemplateOutputPageBeforeExec hook.May 4 2020, 7:36 PM
Jdlrobson renamed this task from [EPIC] Remove SkinTemplateOutputPageBeforeExec hook to Deprecate the SkinTemplateOutputPageBeforeExec hook.May 13 2020, 11:37 PM
Jdlrobson raised the priority of this task from Low to Medium.
Jdlrobson updated the task description. (Show Details)
Restricted Application added a project: Social-Tools. · View Herald TranscriptMay 13 2020, 11:37 PM

I've reviewed the remaining usages of SkinTemplateOutputPageBeforeExec and documented them in T252725. To set some expecations, I'll be opening specific tasks and submitting patches for impacted extensions soon.

In the case of the AddFooterLink hook, I need to make some changes to skins first to support the transition. T251817
I am aiming to deprecate the hook inside 1.36 at latest.

Change 596482 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Deprecate SkinTemplateOutputPageBeforeExec hook

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

Change 596491 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Minerva should source mobile license without indirection

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

Change 596492 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Drop SkinTemplateOutputPageBeforeExec hook usage prior to deprecation

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

Jdlrobson updated the task description. (Show Details)May 14 2020, 6:15 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)May 14 2020, 6:20 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)May 14 2020, 6:22 PM
Jdlrobson updated the task description. (Show Details)May 19 2020, 6:52 PM
Jdlrobson updated the task description. (Show Details)May 19 2020, 7:03 PM
Jdlrobson updated the task description. (Show Details)

Change 597385 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/BlogPage@master] Don't try to suppress categories

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

Jdlrobson updated the task description. (Show Details)May 20 2020, 2:57 PM

Change 597385 merged by jenkins-bot:
[mediawiki/extensions/BlogPage@master] Don't try to suppress categories

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

Jdlrobson updated the task description. (Show Details)May 26 2020, 7:01 PM

Change 596491 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Minerva should source mobile license without indirection

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

Jdlrobson updated the task description. (Show Details)May 28 2020, 6:38 PM

Change 596492 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Drop SkinTemplateOutputPageBeforeExec hook usage prior to deprecation

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

Jdlrobson updated the task description. (Show Details)Jun 2 2020, 10:25 PM
Jdlrobson updated the task description. (Show Details)Jun 2 2020, 11:44 PM
Jdlrobson updated the task description. (Show Details)Jun 3 2020, 7:00 PM
Jdlrobson updated the task description. (Show Details)Jun 4 2020, 11:28 PM
Jdlrobson moved this task from Inbox to OKR Backlog on the User-Jdlrobson board.Jun 9 2020, 11:33 PM
Jdlrobson moved this task from OKR Backlog to Blocked on the User-Jdlrobson board.Jun 9 2020, 11:42 PM
Jdlrobson updated the task description. (Show Details)Sat, Jun 13, 3:49 AM
Ammarpad updated the task description. (Show Details)Mon, Jun 15, 1:18 AM

Change 596482 had a related patch set uploaded (by Ammarpad; owner: Jdlrobson):
[mediawiki/core@master] Deprecate SkinTemplateOutputPageBeforeExec hook

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

Ammarpad updated the task description. (Show Details)Mon, Jun 15, 5:32 PM
Jdlrobson raised the priority of this task from Medium to High.Tue, Jun 16, 6:40 PM
Jdlrobson updated the task description. (Show Details)Mon, Jun 22, 3:06 PM
ovasileva set the point value for this task to 2.Wed, Jun 24, 5:28 PM

Change 596482 merged by jenkins-bot:
[mediawiki/core@master] Deprecate SkinTemplateOutputPageBeforeExec hook

https://gerrit.wikimedia.org/r/c/mediawiki/core/ /596482

Jdlrobson closed this task as Resolved.Mon, Jun 29, 11:19 PM
Jdlrobson updated the task description. (Show Details)