Page MenuHomePhabricator

Drop support for SkinTemplateToolboxEnd in Vector, with suitable replacement
Closed, ResolvedPublic

Description

This hook in core exists for compatibility reasons and is causing integration problems as we push ahead with desktop refresh leading to an UBN (T252906)

Even worse it has been introduced and run in Vector for some time so the hook runs in two different context.

It works around the fact that no other hooks allow modification of the languages and toolbox menus.

I propose we expand the SidebarBeforeOutput hook to allow for this. See POC patches.

Acceptance criteria

Event Timeline

Change 596756 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Treat languages and toolbox like other sidebar links

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

Change 596759 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/ShortUrl@master] Add Vector support

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

Change 596760 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] Drop support for SkinTemplateToolboxEnd

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

Jdlrobson updated the task description. (Show Details)May 15 2020, 9:24 PM
Jdlrobson renamed this task from Deprecate SkinTemplateToolboxEnd in core and find suitable replacement to Drop support for SkinTemplateToolboxEnd in Vector, finding suitable replacement.May 15 2020, 9:29 PM

Alternatively if https://gerrit.wikimedia.org/r/596756 is deemed too risky I'd suggest we add the link explicitly to the projects that use it by an edit to MediaWiki:Sidebar and stop ShortUrl adding to toolbox. Only 31 projects use this extension so that would be easy enough. It also gives editors full control of the toolbox contents.

'bdwikimedia' => true, // T146014
'bhwiki' => true, // T113348
'bnwiki' => true, // T62956
'bnwikisource' => true, // T127968
'eswikibooks' => true, // T96668
'gomwiki' => true, // T206741
'hiwiki' => true,
'hiwikiversity' => true, // T177187
'knwiki' => true, // T97218
'knwikisource' => true, // T189287
'maiwiki' => true, // T125802
'mrwiki' => true, // T103646
'newiki' => true, // T92820
'orwiki' => true,
'orwikisource' => true, // T124429
'orwiktionary' => true, // T103644
'pawiki' => true, // T178919
'sawiki' => true, // T94660
'sawikibooks' => true, // T94660
'sawikiquote' => true, // T94660
'sawikisource' => true, // T94660
'sawiktionary' => true, // T94660
'tawiki' => true,
'tawikibooks' => true,
'tawikinews' => true,
'tawikiquote' => true,
'tawikisource' => true,
'tawiktionary' => true,
'tcywiki' => true, // T150166
'test2wiki' => true,
'urwiki' => true, // T138507
Jdlrobson updated the task description. (Show Details)May 19 2020, 8:12 PM

Would it work for ShortUrl to modify the structured array for sidebar instead, slightly earlier? That seems like a cleaner and easier to support approach than any form of arbitrary HTML.

Moving this on-wiki would imho be a mistake. The wiki should not want to control this. The extension should just work. It applies outside WMF as well.

Adding a link to the sidebar seems trivial and something that should be easy to support in a maintainer manner. If we can't currently, then the sidebar might be a good one to tackle next after the footer for a more structured hook. Although it seems like the sidebar already has it to some extent.

Jdlrobson added a comment.EditedMay 19 2020, 10:08 PM

@Krinkle yes provided https://gerrit.wikimedia.org/r/596756 lands. Right now the sidebar hooks cannot modify toolbox or languages. There is no way with any hook to do so.

https://gerrit.wikimedia.org/r/c/596759 shows how ShortUrl would use the new hook - arguably much cleaner.

An alternative direction would be to follow the example of AddFooterLink and create a new hook e.g. AddSidebarLink to replace the existing sidebar hooks but that greatly increases the scope. I'd much rather do this smaller change first and commit to doing that later.

ovasileva triaged this task as High priority.May 20 2020, 10:06 AM

Change 596756 merged by jenkins-bot:
[mediawiki/core@master] Treat languages and toolbox like other sidebar links

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

Jdlrobson renamed this task from Drop support for SkinTemplateToolboxEnd in Vector, finding suitable replacement to Drop support for SkinTemplateToolboxEnd in Vector, with suitable replacement.May 26 2020, 7:08 PM
Jdlrobson updated the task description. (Show Details)

Change 596759 merged by jenkins-bot:
[mediawiki/extensions/ShortUrl@master] Avoid losing Vector support

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

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

Want to acknowledge the majority of this is done now. Just the formality now of dropping the hook execution in https://gerrit.wikimedia.org/r/c/596760

Jdlrobson moved this task from Needs triage to Hooks on the Vector board.May 29 2020, 1:14 AM

Change 599687 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] Items added with 'TOOLBOX/LANGUAGES' magic strings by hooks should be added to the appropriate section.

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

Change 599753 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/extensions/ShortUrl@master] Remove SkinTemplateToolboxEnd hook workaround for legacy skins

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

Change 599687 merged by jenkins-bot:
[mediawiki/core@master] Links added via SidebarBeforeOutput are exposed to skins using BaseTemplate

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

Jdlrobson changed the task status from Open to Stalled.May 29 2020, 10:10 PM
Jdlrobson updated the task description. (Show Details)

Change 599753 merged by jenkins-bot:
[mediawiki/extensions/ShortUrl@master] Remove SkinTemplateToolboxEnd hook workaround for legacy skins

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

Jdlrobson changed the task status from Stalled to Open.Jun 11 2020, 8:24 PM
Jdlrobson closed this task as Resolved.Jun 11 2020, 10:13 PM
Jdlrobson updated the task description. (Show Details)

Change 596760 abandoned by Jdlrobson:
Drop support for SkinTemplateToolboxEnd

Reason:
No longer needed https://gerrit.wikimedia.org/r/602543

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