On 2014-05-24, 12:12 PM, Bartosz Dziewoński wrote:
First of all, something we all agree on: let's murder skin autodiscovery. I'll submit patches to emit deprecation warnings if a skin using it is found (to master and 1.23 LTS release), and another patch that will remove it entirely (intended for MW 1.24 or MW 1.25).
+1
As for Daniel's concerns about capitalization differences (which are valid regardless of which directory we want to place the skins in), forcing everything to be lowercase is a solution, but one that I don't like very much. Keeping the status quo is also a solution, but clearly no one at all likes this one.
I'm going to name the three names we have 'skin file name' (same as skin directory name), 'skin name' (the one used in $wgValidSkins, saved in user preferences etc., used for site and user JS and CSS pages) and 'proper skin name' (localised one displayed on Special:Preferences).
Let me propose two more ways we could do this. (All of the back-compat concerns below don't apply to new skins, only existing ones that we'd want to retro-fit to the new way. We could also avoid them by not retro-fitting.)
- Instead of switching file name to lowercase, why not switch skin
name to CamelCase? We'd have a maintenance script to update user preferences in the database, and a little back-compat layer to allow skins to define non-canonical names they want to respond to (for site admins who can't run the maintenance script).
This will, however, break back-compat for JS user scripts (that implement different behavior per-skin), as well as very poorly written PHP-side code (which should have no reason to care about skins). This might be acceptable.
- Always compare skin names case-insensitively (which is easy if they're all-ASCII), in 'useskin', in database user preferences data,
everywhere – and use CamelCased names in "user-visible" areas. We could keep the canonical skin names the way they are in existing skins for back-compat of user preferences, but site admins would be able to set $wgDefaultSkin = 'CologneBlue' even if the canonical skin name was 'cologneblue'.
As far as I can tell this will not break back-compat at all, and would let us move forward leaving very little tech debt behind.
The idea of "migrating" a pile of keys in the first makes me shudder.
In both of these cases please keep some other areas in mind: * i18n message keys like skinname-{$skinname} and {$skinname}-action-viewsource (which the i18n system as set a standard of always using lower case keys) * Site skin scripts and styles MediaWiki:{ucfirst($skinname)}.css + MediaWiki:{ucfirst($skinname)}.js and user scripts and styles User:X/{$skinname}.css + MediaWiki:{ucfirst($skinname)}.js (these would be ridiculously hard to just "migrate".
So either way even if you try replacing skinname with SkinName, the lower case skinname still sticks around.
I am fine with the idea of letting &useskin= and $wgDefaultSkin (and even $wgSkipSkins) work case insensitively (though I wouldn't encourage the actual use of uppercase there). However $wgValidSkins isn't something should become case insensitive, attempting that for array keys is asking for bugs. Same for putting non lowercase keys in the database and trying to make them case insensitive. The easiest way to make &useskin=, $wgDefaultSkin, and $wgSkipSkins case insensitive is to normalize all skin keys from them to lowercase (which is actually only a few lines in Skin.php) and then as a "breaking change" say we're forbidding non-lowercase keys to $wgValidSkins (which should be rather simple since I haven't seen a single skin yet doing that which would actually be broken by such a change).
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]