Page MenuHomePhabricator
Create Task
Maniphest T345039

SkinAfterBottomScripts is called twice on MediaWiki 1.40.0
Closed, ResolvedPublicBUG REPORT
Actions

Description

Steps to replicate the issue (include links if applicable):

Use following code in LocalSettings.php:

$wgHooks['SkinAfterBottomScripts'][] = static function ( $skin, &$text ) {
       echo wfGetAllCallers(20) . "\n\n";
};

What happens?:

wfIndexMain/MediaWiki->run/MediaWiki->main/OutputPage->output/SkinTemplate->outputPage/SkinMustache->generateHTML/OutputPage->tailElement/MediaWiki\HookContainer\HookRunner->onSkinAfterBottomScripts/MediaWiki\HookContainer\HookContainer->run/MediaWiki\HookContainer\HookContainer->callLegacyHook/{closure}

wfIndexMain/MediaWiki->run/MediaWiki->main/OutputPage->output/SkinTemplate->outputPage/SkinMustache->generateHTML/OutputPage->tailElement/OutputPage->getBottomScripts/MediaWiki\HookContainer\HookRunner->onSkinAfterBottomScripts/MediaWiki\HookContainer\HookContainer->run/MediaWiki\HookContainer\HookContainer->callLegacyHook/{closure}

What should have happened instead?:

Expected that the hook is only called once.

Software version (skip for WMF-hosted wikis like Wikipedia):
MediaWiki 1.40.0

Other information (browser name/version, screenshots, etc.):

Details

SubjectRepoBranchLines +/-
Do not run SkinAfterBottomScripts hook twice unconditionallymediawiki/coreREL1_39+3 -8
Do not run SkinAfterBottomScripts hook twice unconditionallymediawiki/coreREL1_40+3 -8
Do not run SkinAfterBottomScripts hook twice unconditionallymediawiki/coremaster+3 -8
Customize query in gerrit

Related Objects

Event Timeline

Nikerabbit created this task.Aug 27 2023, 10:17 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 27 2023, 10:17 AM
Ammarpad subscribed.Aug 27 2023, 11:32 AM

The duplicate call was introduced in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/895791. Not sure why that was necesarry.

Reedy added a project: MW-1.40-release.Aug 27 2023, 2:23 PM
Fomafix subscribed.Aug 27 2023, 8:35 PM

rMW11e0fba3b0cd: Deprecate global function wfReportTime() didn't introduce a duplicate call of SkinAfterBottomScripts. The call was just moved to the beginning of getBottomScripts to include the execution time of this call in the elapsed time calculation.

Ammarpad added a comment.Aug 28 2023, 5:15 AM

Oh yes, it was actually introduced earlier in rMW95a4189814e2: Prepare for deprecation of Skin::bottomScripts.

gerritbot added a comment.Aug 28 2023, 5:23 AM

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

[mediawiki/core@master] Do not run SkinAfterBottomScripts hook twice unconditionally

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

gerritbot added a project: Patch-For-Review.Aug 28 2023, 5:23 AM
Fomafix added a project: MW-1.39-release.Aug 28 2023, 8:10 AM
Fomafix added a subscriber: Jdlrobson.

rMW95a4189814e2: Prepare for deprecation of Skin::bottomScripts is already included in MediaWiki 1.39.

Krinkle removed a project: MediaWiki-Core-Hooks.Aug 28 2023, 2:05 PM
gerritbot added a comment.Sep 7 2023, 6:16 PM

Change 952569 merged by jenkins-bot:

[mediawiki/core@master] Do not run SkinAfterBottomScripts hook twice unconditionally

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

Maintenance_bot removed a project: Patch-For-Review.Sep 7 2023, 6:30 PM
Jdlrobson closed this task as Resolved.Sep 7 2023, 6:51 PM
Jdlrobson claimed this task.
Jdlrobson added a project: Web-Team-Backlog (Web Team FY2023-24 Q1 Sprint 5).

Should be fixed now.

Jdlrobson edited projects, added Web-Team-Backlog (Web Team FY2023-24 Q1 Sprint 6); removed Web-Team-Backlog (Web Team FY2023-24 Q1 Sprint 5).Sep 7 2023, 6:52 PM
ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.26; 2023-09-12).Sep 7 2023, 7:00 PM
Krinkle reopened this task as Open.EditedSep 7 2023, 10:30 PM
Krinkle subscribed.

I believe neither 1.40 nor 1.39 was fixed. Is that right?

Jdlrobson removed a project: Web-Team-Backlog (Web Team FY2023-24 Q1 Sprint 6).Sep 11 2023, 5:09 PM
Jdlrobson added a project: Reading-Web-Third-Party-Support.
Nikerabbit added a comment.Sep 19 2023, 8:43 AM

Unfortunately the patch doesn't backport cleanly from Gerrit UI.

gerritbot added a comment.Sep 19 2023, 12:36 PM

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

[mediawiki/core@REL1_40] Do not run SkinAfterBottomScripts hook twice unconditionally

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

gerritbot added a project: Patch-For-Review.Sep 19 2023, 12:36 PM
gerritbot added a comment.Sep 19 2023, 12:40 PM

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

[mediawiki/core@REL1_39] Do not run SkinAfterBottomScripts hook twice unconditionally

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

Reedy subscribed.Sep 19 2023, 12:40 PM
In T345039#9177557, @Nikerabbit wrote:

Unfortunately the patch doesn't backport cleanly from Gerrit UI.

Backports cleanly to REL1_40 locally...

REL1_39 needed a little more effort...

Reedy updated the task description. (Show Details)Sep 19 2023, 12:40 PM
gerritbot added a comment.Sep 19 2023, 2:28 PM

Change 958924 merged by jenkins-bot:

[mediawiki/core@REL1_40] Do not run SkinAfterBottomScripts hook twice unconditionally

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

ReleaseTaggerBot added a project: MW-1.40-notes.Sep 19 2023, 3:00 PM
gerritbot added a comment.Sep 19 2023, 4:50 PM

Change 958925 merged by jenkins-bot:

[mediawiki/core@REL1_39] Do not run SkinAfterBottomScripts hook twice unconditionally

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

ReleaseTaggerBot added a project: MW-1.39-notes.Sep 19 2023, 5:00 PM
Maintenance_bot removed a project: Patch-For-Review.Sep 19 2023, 5:10 PM
Ammarpad closed this task as Resolved.Sep 20 2023, 9:22 AM
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