Page MenuHomePhabricator

Hard deprecate Skin::setupSkinUserCss
Closed, ResolvedPublic

Description

Having this around and not throwing hard deprecation notices is confusing. This has been deprecated since 1.32

https://codesearch.wmcloud.org/search/?q=setupSkinUserCss&i=nope&files=&repos=

Event Timeline

The hilarious bit here is I'm not even sure any of these skins (or othe skins I've looked at) actually use it for anything.

They basicaly all just call paren::setupSkinUserCss, and even if they have anything else in the function, it could just as easily be in initPage...

Pretty sure they all have it because monobook/example have/had it. Kick it out of those and the rest should follow.

Yep :) I know I've added it in the past hahah. Will be nice to remove this confusion.

I think we can start with hard deprecation and let the removal be in 1.37

Ammarpad renamed this task from Hard deprecate (remove?) Skin::setupSkinUserCss to Hard deprecate Skin::setupSkinUserCss.Jul 28 2020, 5:23 AM

Change 618498 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/skins/Modern@master] Modern: Remove usage of deprecated Skin::setupSkinUserCss

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

Change 618503 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/skins/Timeless@master] Remove Skin::setupSkinUserCss call

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

Change 618503 merged by jenkins-bot:
[mediawiki/skins/Timeless@master] Remove Skin::setupSkinUserCss call

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

Change 618498 merged by jenkins-bot:
[mediawiki/skins/Modern@master] Modern: Remove usage of deprecated Skin::setupSkinUserCss

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

Change 619142 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/skins/CologneBlue@master] Remove usage of deprecated Skin::setupSkinUserCss

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

Change 619143 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/skins/Nostalgia@master] Remove usage of Skin::setupSkinUserCss

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

Change 619144 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] Deprecate Skin::setupSkinUserCss

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

Change 619143 merged by jenkins-bot:
[mediawiki/skins/Nostalgia@master] Remove usage of Skin::setupSkinUserCss

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

Change 619142 merged by jenkins-bot:
[mediawiki/skins/CologneBlue@master] Remove usage of deprecated Skin::setupSkinUserCss

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

Change 619144 merged by jenkins-bot:
[mediawiki/core@master] Deprecate Skin::setupSkinUserCss

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

@Ammarpad The removal of the call to setupSkinUserCss() in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/619144/2/includes/OutputPage.php
broke many skins from the following list as these no longer load their stylesheets:
https://codesearch.wmcloud.org/search/?q=setupSkinUserCss&i=nope&files=&repos=

The deprecation warning inserted here is ineffective as the overridden methods aren't called, which in turn won't call this method.
Consequently this patch immediately removes functionality without warning, contrary to the deprecation policy.
To properly warn when a skin overrides the deprecated method reflection is necessary.
Fix: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/622254

Thanks for flagging this.

According to https://www.mediawiki.org/wiki/Stable_interface_policy#Deprecation "Hard deprecated code MAY act as no-ops instead of actually functioning, though this is not recommended.".
This might be a case of where this case is recommended.

In this case it's obvious the functionality is broken by the failure of the stylesheet to load. That should be a sufficient enough warning IMO to a skin developer. Possibly an email to wikitech-l would also help here.

Change 642847 had a related patch set uploaded (by Tim Starling; owner: Tim Starling):
[mediawiki/core@master] Revert "Deprecate Skin::setupSkinUserCss"

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

Ammarpad added a subscriber: Ammarpad.

Change 642847 merged by jenkins-bot:
[mediawiki/core@master] Revert "Deprecate Skin::setupSkinUserCss"

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

Ammarpad claimed this task.

Hard deprecated in another way

Change 644931 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Emit deprecation warning for deprecated overrides.

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

Change 644931 merged by jenkins-bot:
[mediawiki/core@master] Emit deprecation warning for deprecated overrides.

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