Page MenuHomePhabricator

Fix or archive skin using deprecated Skin::setupSkinUserCss() method
Closed, ResolvedPublicBUG REPORT

Description

Skin::setupSkinUserCss() was a step in adding the stylesheets to the generated page. Skins that used to override this method now (since 1.36-wmf.5) appear completely unstyled.
The task is to make all skins in the ecosystem operational again. The deadline for this task is the first 1.36-beta release.

History

Soft deprecated in 1.32, hard deprecated in 1.36, planned to be removed in 1.37 (T257990#6336906).
It was erroneously removed in 1.36-wmf.5 (patch), breaking non-WMF skins in violation of the Stable interface policy.

Impacts 26 skins and extensions:
https://codesearch.wmcloud.org/search/?q=setupSkinUserCss&i=nope&files=&repos=librewiki/Liberty-MW-Skin,ProfessionalWiki/chameleon,Skin:Amethyst,Skin:Bouquet,Skin:Cosmos,Skin:Daddio,Skin:DeskMessMirrored,Skin:Dusk,Skin:DuskToDawn,Skin:erudite,Skin:EUCopyrightCampaignSkin,Skin:Gamepress,Skin:HasSomeColours,Skin:Mask,Skin:Material,Skin:mediawiki-strapping,Skin:Metrolook,Skin:Nimbus,Skin:p2wiki,Skin:Refreshed,Skin:Tempo,Skin:Truglass,Skin:webplatform,Skin:WPtouch,thingles/foreground,wiki-chan/ModernSkylight
(Note: URL filters out unaffected skins - some skins extending the function without doing anything.

Ignored steps of the Stable Interface Policy
  • "Code MUST emit hard deprecation notices for at least one major MediaWiki version before being removed. It is RECOMMENDED to emit hard deprecation notices for at least two major MediaWiki versions."
  • "The removal MUST also be mentioned in the relevant RELEASE-NOTES file"
  • "Developers MAY email wikitech-l or mediawiki-l about the soft or hard deprecation depending on severity."
  • "[...] may be necessary to make breaking changes [...] In such a case, developers MUST email wikitech-l ahead of time, explaining why deprecation is not possible or not reasonable, and providing an opportunity for affected parties to raise concerns and propose alternatives."
Does not apply:
  • "The deprecation process may be bypassed for code that is unused within the MediaWiki ecosystem. [...] can be searched using the code search tool."
  • "Additionally, in some rare cases, it may be necessary to make breaking changes without deprecation it in a major MediaWiki version beforehand, because the old behavior cannot reasonably be emulated. In such a case, developers MUST email wikitech-l ahead of time, explaining why deprecation is not possible or not reasonable, and providing an opportunity for affected parties to raise concerns and propose alternatives."
See also

Formalization of the solution: T267085: Clarify deprecation of method overrides in the stable interface policy

Details

ProjectBranchLines +/-Subject
mediawiki/skins/Truglassmaster+10 -8
mediawiki/skins/TempoREL1_36+10 -8
mediawiki/skins/DeskMessMirroredREL1_36+14 -12
mediawiki/skins/BouquetREL1_36+13 -16
mediawiki/skins/Tempomaster+10 -8
mediawiki/skins/DeskMessMirroredmaster+14 -12
mediawiki/skins/Bouquetmaster+13 -16
mediawiki/skins/Duskmaster+13 -13
mediawiki/skins/DuskToDawnmaster+13 -16
mediawiki/skins/Refreshedmaster+12 -12
mediawiki/skins/Gamepressmaster+35 -24
mediawiki/skins/eruditemaster+23 -13
mediawiki/skins/GuMaxDDmaster+4 -13
mediawiki/skins/WPtouchmaster+19 -17
mediawiki/skins/Nimbusmaster+22 -15
mediawiki/skins/p2wikimaster+16 -23
mediawiki/skins/Schulenburgmaster+0 -12
mediawiki/skins/Amethystmaster+28 -37
mediawiki/extensions/Blackoutmaster+0 -12
mediawiki/skins/HasSomeColoursmaster+131 -501
mediawiki/skins/Maskmaster+15 -19
mediawiki/skins/Materialmaster+13 -15
mediawiki/skins/Metrolookmaster+5 -4
mediawiki/skins/Cosmosmaster+57 -83
mediawiki/coremaster+20 -3
mediawiki/skins/BlueSpiceCalummaREL1_35+19 -15
mediawiki/skins/BlueSpiceCalummamaster+19 -15
mediawiki/coremaster+21 -2
mediawiki/skins/apexmaster+11 -15
mediawiki/skins/apexmaster+6 -5
Show related patches Customize query in gerrit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

In future my hope is to get a MediaWiki skin site up and running to showcase all the skins in our ecosystem and make more incentive for skin development.

That would be great to have, and no doubt it'd help end-users in picking the right skin for their site.

I've created a wiki (https://skins.toolforge.org/wiki/Main_Page) with all stable skins and also skins under wmf version control (Gerrit) that work with modern versions of MediaWiki.

Change 639933 merged by jenkins-bot:
[mediawiki/skins/BlueSpiceCalumma@master] Replace usage of since 1.32 deperecated Skin::setupSkinUserCss

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

Change 639970 merged by jenkins-bot:
[mediawiki/skins/BlueSpiceCalumma@REL1_35] Replace usage of since 1.32 deperecated Skin::setupSkinUserCss

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

Change 640325 had a related patch set uploaded (by Jack Phoenix; owner: Jack Phoenix):
[mediawiki/skins/Mask@master] Update for MediaWiki 1.35+

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

Jdlrobson lowered the priority of this task from High to Medium.Nov 16 2020, 7:40 PM

1.36 is a while away so I think medium better reflects this task. I am confident we'll get skins fixed or archived before then.

Change 639875 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Nimbus@master] Make Nimbus skin 1.36 compatible

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

Jdlrobson renamed this task from Unmaintained skins broken by hard deprecation / disabling of Skin::setupSkinUserCss() method to Fix or archive skin using deprecated Skin::setupSkinUserCss() method.Nov 24 2020, 11:44 PM
Jdlrobson removed a project: Platform Engineering.
Jdlrobson edited projects, added Platform Engineering; removed MediaWiki-skins-Daddio.

Daddio is being archived so untagging (T267455)

Platform engineering have made sure these skins are not broken now so untagging them and renamed.

Change 644649 had a related patch set uploaded (by Universal Omega; owner: Universal Omega):
[mediawiki/skins/Cosmos@master] Cleanup skin.json and SkinCosmos.php

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

Change 644649 merged by jenkins-bot:
[mediawiki/skins/Cosmos@master] Cleanup skin.json and SkinCosmos.php

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

Change 637635 merged by jenkins-bot:
[mediawiki/skins/Metrolook@master] SkinMetrolook: Migrate deprecated setupSkinUserCss() to getDefaultModules()

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

Change 637620 merged by jenkins-bot:
[mediawiki/skins/Material@master] Material.skin: Migrate setupSkinUserCss() to skin.json

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

Change 640325 merged by jenkins-bot:
[mediawiki/skins/Mask@master] Update for MediaWiki 1.35+

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

Change 650274 had a related patch set uploaded (by Isarra; owner: Isarra):
[mediawiki/skins/HasSomeColours@master] Use updated everything

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

Change 650274 merged by jenkins-bot:
[mediawiki/skins/HasSomeColours@master] Use updated everything

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

Change 650538 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Blackout@master] The setupSkinUserCss method is deprecated and not ever run

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

Change 650538 merged by jenkins-bot:
[mediawiki/extensions/Blackout@master] The setupSkinUserCss method is deprecated and not ever run

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

Change 650541 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Amethyst@master] Modernize Amethyst skin code

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

Change 650541 merged by jenkins-bot:
[mediawiki/skins/Amethyst@master] Modernize Amethyst skin code

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

Change 651785 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Schulenburg@master] Drop unused setupSkinUserCss

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

Change 651785 merged by jenkins-bot:
[mediawiki/skins/Schulenburg@master] Drop unused setupSkinUserCss

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

Change 651815 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/WPtouch@master] Skin::setupSkinUserCss() method is deprecated

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

Change 651828 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/GuMaxDD@master] Add support for 1.35

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

Change 651831 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/p2wiki@master] Should support 1.35 / 1.36

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

Change 651831 merged by jenkins-bot:
[mediawiki/skins/p2wiki@master] Should support 1.35 / 1.36

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

Change 651815 merged by jenkins-bot:
[mediawiki/skins/WPtouch@master] Skin::setupSkinUserCss() method is deprecated

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

Change 639875 merged by jenkins-bot:
[mediawiki/skins/Nimbus@master] Make Nimbus skin 1.36 compatible

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

Jdlrobson reopened this task as Open.
Jdlrobson claimed this task.
Jdlrobson removed a project: Nimbus.

Change 651828 merged by Jdlrobson:
[mediawiki/skins/GuMaxDD@master] Add support for 1.35

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

Change 654335 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/erudite@master] Add 1.35 support

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

Change 654335 merged by jenkins-bot:
[mediawiki/skins/erudite@master] Add 1.35 support

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

Change 639884 merged by jenkins-bot:
[mediawiki/skins/Gamepress@master] Upgrade SkinGamepress to take advantage of new skin capabilities

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

Jdlrobson added a subscriber: Osnard.

If any volunteers want to help fix these skins prior to the 1.36 release I will happily review. It would be great to ship with a full set of working skins. If not we should retag with MW-1.37-release

@Osnard @ashley as the maintainers of the remaining skins what do you think?

Change 676857 had a related patch set uploaded (by Jack Phoenix; author: Jack Phoenix):

[mediawiki/skins/Bouquet@master] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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

Change 676858 had a related patch set uploaded (by Jack Phoenix; author: Jack Phoenix):

[mediawiki/skins/Dusk@master] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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

Change 676859 had a related patch set uploaded (by Jack Phoenix; author: Jack Phoenix):

[mediawiki/skins/DuskToDawn@master] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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

Change 676860 had a related patch set uploaded (by Jack Phoenix; author: Jack Phoenix):

[mediawiki/skins/Refreshed@master] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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

Change 676861 had a related patch set uploaded (by Jack Phoenix; author: Jack Phoenix):

[mediawiki/skins/Truglass@master] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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

@Osnard @ashley as the maintainers of the remaining skins what do you think?

Sounds good! As per @gerritbot's comments above, I've updated the skins I maintain to ditch the setupSkinUserCss method altogether where it was present and thus also subsequently bumped the version requirement from the EOL'd 1.34 to 1.35 for the skins which did not yet require 1.35 (Refreshed and Truglass were already requiring 1.35, the others weren't). While I did quickly test these patches (as in I loaded the Main Page of my 1.35 box before and after the changes and verified that everything roughly looks the same and no styles etc. are missing), further testing and code review is always very much welcome!

...Having typed this comment, I realize I forgot to touch DeskMessMirrored. Unlike the aforementioned skins, that one has some Internet Explorer specific fixes in its setupSkinUserCss method, and I'm not quite sure how to port those over.

Change 676860 merged by jenkins-bot:

[mediawiki/skins/Refreshed@master] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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

Change 676859 merged by jenkins-bot:

[mediawiki/skins/DuskToDawn@master] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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

Change 676858 merged by jenkins-bot:

[mediawiki/skins/Dusk@master] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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

Hey there, should this be moved to 1.37? The cut for 1.36 has happened, and 1.36.0-rc.0 will be cut in a fortnight or so, after which feature changes shouldn't be landed and back-ported.

...Having typed this comment, I realize I forgot to touch DeskMessMirrored. Unlike the aforementioned skins, that one has some Internet Explorer specific fixes in its setupSkinUserCss method, and I'm not quite sure how to port those over.

can you not use SkinTemplate::initPage ?

Hey there, should this be moved to 1.37?

This ticket should be resolved when 1.36 is released. The idea was to make sure skins were not broken for the release. The remaining 2 skins can probably be considered unsupported for now (sounds like @ashley plans to fix DeskLessMirrorered).

The cut for 1.36 has happened, and 1.36.0-rc.0 will be cut in a fortnight or so, after which feature changes shouldn't be landed and back-ported.

Did I miss a pencil's down email on wikitech-l warning about this? I can't see any announcement in my inbox for 1.36 and I usually rely on those emails for escalating priorities.

Change 676857 merged by jenkins-bot:

[mediawiki/skins/Bouquet@master] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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

Change 679713 had a related patch set uploaded (by Jack Phoenix; author: Jack Phoenix):

[mediawiki/skins/DeskMessMirrored@master] Modernize CSS loading a bit, ditch setupSkinUserCss() and load IE fixes in initPage(), require MW 1.35+

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

Change 679717 had a related patch set uploaded (by Jack Phoenix; author: Jack Phoenix):

[mediawiki/skins/Tempo@master] Ditch setupSkinUserCss() for forwards-compatibility (we already require MW 1.35 in skin.json)

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

Change 679826 had a related patch set uploaded (by Jdlrobson; author: Jack Phoenix):

[mediawiki/skins/Bouquet@REL1_36] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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

Change 679827 had a related patch set uploaded (by Jdlrobson; author: Jack Phoenix):

[mediawiki/skins/DeskMessMirrored@REL1_36] Modernize CSS loading a bit, ditch setupSkinUserCss() and load IE fixes in initPage(), require MW 1.35+

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

Change 679828 had a related patch set uploaded (by Jdlrobson; author: Jack Phoenix):

[mediawiki/skins/Tempo@REL1_36] Ditch setupSkinUserCss() for forwards-compatibility (we already require MW 1.35 in skin.json)

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

Change 679713 merged by jenkins-bot:

[mediawiki/skins/DeskMessMirrored@master] Modernize CSS loading a bit, ditch setupSkinUserCss() and load IE fixes in initPage(), require MW 1.35+

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

Change 679717 merged by jenkins-bot:

[mediawiki/skins/Tempo@master] Ditch setupSkinUserCss() for forwards-compatibility (we already require MW 1.35 in skin.json)

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

Change 679826 merged by jenkins-bot:

[mediawiki/skins/Bouquet@REL1_36] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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

Change 679827 merged by jenkins-bot:

[mediawiki/skins/DeskMessMirrored@REL1_36] Modernize CSS loading a bit, ditch setupSkinUserCss() and load IE fixes in initPage(), require MW 1.35+

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

Change 679828 merged by jenkins-bot:

[mediawiki/skins/Tempo@REL1_36] Ditch setupSkinUserCss() for forwards-compatibility (we already require MW 1.35 in skin.json)

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

According to T238803 the fixcopyright skin is being retired, so all skins have now been fixed and this can be considered done.
I've cherry picked to REL1_36 where needed.

Change 676861 merged by jenkins-bot:

[mediawiki/skins/Truglass@master] Ditch the Skin::setupSkinUserCss method(), require MW 1.35+

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