Page MenuHomePhabricator

Hard deprecate Skin::bottomScripts(), move use of SkinAfterBottomScripts hook to OutputPage::getBottomScripts()
Closed, ResolvedPublic

Description

Ran into this at [[mw:Thread:Project:Support desk/How to get Piwik code into MobileFrontend?]] and, as a result found this comment right above where the hook is called:

// TODO and the suckage continues. This function is really just a wrapper around
// OutputPage::getBottomScripts() which takes a Skin param. This should be
// cleaned up at some point

Could deprecating Skin::bottomScripts(), and moving the use of the SkinAfterBottomScripts hook to OutputPage::getBottomScripts() fix this?


Version: 1.22.2
Severity: normal

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:54 AM
bzimport set Reference to bz60846.
bzimport added a subscriber: Unknown Object (MLST).

This doesn't really change functionality and it fixes a problem with some extensions (interaction of MobileFrontend and Piwik in this case.

If it is ok, I'd like to target the next 1.22 point release.

Removing target milestone that was in the past.

If you want this in a specific release, have a good reason AND you are willing to find resources to fix this bug, feel free to change it to something appropriate.

Krinkle removed a subscriber: Krinkle.
Krinkle added a subscriber: Krinkle.
Krinkle removed a subscriber: Krinkle.
TheDJ added a subscriber: TheDJ.

I note that mobilefrontend now also calls the Skin's wrapper around bottom scripts.

In order to deprecate this:

  1. replace direct inclusion of reporttime and bottomscripts, with printTrail() in MobileFrontend/includes/skins/MinervaTemplate.php~
  2. Move the work of Skin::bottomScripts() into printTrail() of BaseTemplate
  3. Make sure SkinTemplate sets bottomscripts of the template to empty string, for backwards compatibility
TheDJ set Security to None.
Krinkle lowered the priority of this task from Medium to Low.Oct 12 2019, 10:45 PM
Krinkle moved this task from Inbox to Radar on the Performance-Team board.
Krinkle edited projects, added Performance-Team (Radar); removed Performance-Team.
Krinkle moved this task from Limbo to Watching on the Performance-Team (Radar) board.
Krinkle moved this task from Inbox to Accepted Enhancement on the MediaWiki-ResourceLoader board.

Change 777826 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Timeless@master] Timeless should only generate body of page

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

Change 777827 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Prepare for deprecation of Skin::bottomScripts

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

Change 777828 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Deprecate use of bottomscripts

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

For SkinMustache skins we made it so that skin HTML generation was scoped to the content of the body tag. I think it makes sense to extend this to SkinTemplate skins. As demonstrated in Timeless, this would mean setting a skin option and removing several lines of code. Given this could potentially impact a lot of skins, we would likely need to leave existing code for at least 2 release cycles to allow for migration.

Jdlrobson raised the priority of this task from Low to Medium.Apr 6 2022, 3:41 PM
Jdlrobson renamed this task from Deprecate Skin::bottomScripts(), move use of SkinAfterBottomScripts hook to OutputPage::getBottomScripts() to Hard deprecate Skin::bottomScripts(), move use of SkinAfterBottomScripts hook to OutputPage::getBottomScripts().Apr 6 2022, 3:51 PM

Change 777827 merged by jenkins-bot:

[mediawiki/core@master] Prepare for deprecation of Skin::bottomScripts

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

Change 777826 merged by jenkins-bot:

[mediawiki/skins/Timeless@master] Timeless should only generate body of page

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

Change 785114 had a related patch set uploaded (by Ammarpad; author: Ammarpad):

[mediawiki/skins/Nostalgia@master] Nostalgia: Generate body of page only

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

Change 777828 merged by jenkins-bot:

[mediawiki/core@master] Deprecate use of bottomscripts

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

Change 785114 merged by jenkins-bot:

[mediawiki/skins/Nostalgia@master] Nostalgia: Generate body of page only

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