From 442359e48992d87c02eae3b97bf1c5954745cf42 Mon Sep 17 00:00:00 2001 From: Daniel Friesen Date: Sun, 26 Dec 2010 14:15:27 +0000 Subject: [PATCH] Changing the skin loader to load classes using the pattern "Skin{$skinName}" instead of `"Skin" . ucfirst($key)` as the current behavior has been causing bugs with extension based skins attempting to use the autoloader to load their skins. Under current behavior for "MonoBook" the skin loader will load "SkinMonobook" instead of "SkinMonoBook". Because the skin system loads from skins/ when it can't find a skin and php's class system is case insensitive by a fluke MonoBook has been fine despite the skin loader trying to load SkinMonobook despite the class being named SkinMonoBook. However our autoloader is case-sensitive, and as a result if you try to name your extension based skin class something like SkinStereoBook the skin will break with a cryptic error message because the skin loader attempts to load SkinStereobook, doesn't find it in the autoloader, then throws a php error when it tries to load skins/StereoBook.php and can't find the file. We have also been using the $skinName to generate the name of the file to load from skins/ so this value is already expected to be safe for use in this way. --- RELEASE-NOTES | 3 +++ includes/DefaultSettings.php | 5 ++++- includes/Skin.php | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 0b1ea7b878..0449c2a567 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -23,6 +23,9 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN WantedPages::getQueryInfo . This may break older extensions. * $wgUseCombinedLoginLink controls whether to output a combined login / create account link in the personal bar, or to output separate login and create account links +* Skin names are no longer created based on a ucfirst version of the key in $wgValidSkinNames but now + the value. This means for $wgValidSkinNames["monobook"] = "MonoBook"; the skin + loader will no longer try loading SkinMonobook and will instead load SkinMonoBook. === New features in 1.18 === * Added a special page, disabled by default, that allows users with the diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 2a8cf19e30..7368d60592 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -4460,7 +4460,10 @@ $wgParserOutputHooks = array(); /** * List of valid skin names. - * The key should be the name in all lower case, the value should be a display name. + * The key should be the name in all lower case, the value should be a properly + * cased name for the skin. This value will be prefixed with "Skin" to create the + * class name of the skin to load, and if the skin's class cannot be found through + * the autoloader it will be used to load a .php file by that name in the skins directory. * The default skins will be added later, by Skin::getSkinNames(). Use * Skin::getSkinNames() as an accessor if you wish to have access to the full list. */ diff --git a/includes/Skin.php b/includes/Skin.php index a9dbae207e..7da81b4b10 100644 --- a/includes/Skin.php +++ b/includes/Skin.php @@ -142,7 +142,7 @@ class Skin extends Linker { $skinNames = Skin::getSkinNames(); $skinName = $skinNames[$key]; - $className = 'Skin' . ucfirst( $key ); + $className = "Skin{$skinName}"; # Grab the skin class and initialise it. if ( !class_exists( $className ) ) { -- 2.20.1