Page MenuHomePhabricator

Deprecate BaseTemplateToolbox hook
Closed, ResolvedPublic

Description

The BaseTemplateToolbox hook adds logic to the rendering layer.
It's used in a few places:
https://codesearch.wmflabs.org/search/?q=BaseTemplateToolbox&i=nope&files=&repos=

With T252917 and @Ammarpad modification the SidebarBeforeOutput hook will be usable instead.

Let's deprecate the hook and move usages to SidebarBeforeOutput.

Acceptance criteria

  • All extensions in subtasks should use SidebarBeforeOutput (Vector) instead of the BaseTemplateToolbox. This should retain support with Modern, Monobook and Cologne Blue.
  • Vector should not call BaseTemplate::getToolbox
  • Soft deprecate BaseTemplate::getToolbox in core
  • Mark BaseTemplateToolbox hook as deprecated.

Related Objects

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 22 2020, 8:23 PM
Jdlrobson updated the task description. (Show Details)May 27 2020, 6:21 PM
Restricted Application added a subscriber: Liuxinyu970226. · View Herald TranscriptMay 27 2020, 6:21 PM

Hi @Jdlrobson, there's some problem with the new hook. It seems the TOOLBOX/LANGUAGES magic strings are not really recognized as the items are not added. Using custom heading works, but that's not wanted in most cases.

@Ammarpad can you share the patch you are having issues with that's not working? I'll take a look.

Ammarpad added a comment.EditedMay 28 2020, 8:32 PM

Ok, I see it does not work well in all skins. In r596759 for MediaWiki-extensions-ShortUrl you had to use SkinTemplateToolboxEnd hook ( for 'legacy skins') even though you have also used SidebarBeforeOutput hook. But in T252917, you're also on the way to remove the former.

r596759 for ShortUrl and r598958 for UrlShorterner both do not seem to add the link if you remove SkinTemplateToolboxEnd hook.

So what's the status of SkinTemplateToolboxEnd? and what are the 'legacy skins'?

Jdlrobson added a comment.EditedMay 28 2020, 9:08 PM

Hi @Ammarpad, thanks for flagging. Sounds like we have a chicken egg problem. SkinTemplateToolboxEnd needs to be deprecated. In ShortUrl I added support for SidebarBeforeOutput but kept the SkinTemplateToolboxEnd hook around while we update CologneBlue, Monobook, Timeless and Modern.

So I guess as we do this we need to use both for now, and then once that's done, update CologneBlue, Monobook, Timeless and Modern to stop using SkinTemplateToolboxEnd.

We'll then mark the SkinTemplateToolboxEnd hook as deprecated and remove all the SkinTemplateToolboxEnd hooks via the normal deprecation process to allow other skins to catch up.

Does that make sense?

Thanks for the clarification. Yes, that's OK.

Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)May 29 2020, 10:08 PM

Change 602543 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/skins/Vector@master] Vector should not call BaseTemplate::getToolbox

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

Jdlrobson triaged this task as High priority.Jun 5 2020, 8:49 PM

Change 602796 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/extensions/Cargo@master] Use SidebarBeforeOutput hook to add toolbox item

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

Change 602796 merged by jenkins-bot:
[mediawiki/extensions/Cargo@master] Use SidebarBeforeOutput hook to add toolbox item

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

Jdlrobson changed the task status from Open to Stalled.Jun 10 2020, 11:59 PM

This is stalled on T253685 now.

Jdlrobson changed the task status from Stalled to Open.Jun 11 2020, 8:19 PM
Jdlrobson updated the task description. (Show Details)

Change 602543 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Vector should not call BaseTemplate::getToolbox

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

Jdlrobson updated the task description. (Show Details)
Jdlrobson removed Jdlrobson as the assignee of this task.Jun 11 2020, 10:29 PM
Jdlrobson added a project: patch-welcome.
Ammarpad claimed this task.Jun 12 2020, 2:19 PM

Change 605292 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/skins/Vector@master] Simplify Vector buildSidebar() method

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

Change 605309 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] Soft deprecate BaseTemplate::getToolbox()

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

Change 605309 merged by jenkins-bot:
[mediawiki/core@master] Soft deprecate BaseTemplate::getToolbox()

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

Change 605292 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Simplify Vector buildSidebar() method

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

Jdlrobson updated the task description. (Show Details)
Ammarpad updated the task description. (Show Details)Jun 13 2020, 7:41 AM
Ammarpad updated the task description. (Show Details)Jun 13 2020, 7:49 AM

Change 605351 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] Hard deprecate BaseTemplateToolbox hook

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

Change 605351 merged by jenkins-bot:
[mediawiki/core@master] Hard deprecate BaseTemplateToolbox hook

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

Jdlrobson closed this task as Resolved.Jun 13 2020, 3:49 PM
Jdlrobson updated the task description. (Show Details)