Last modified: 2010-05-15 16:03:18 UTC
One gets harmless errors that a stylesheet was given with content-type text/html on some browsers (like Safari) because a non-existent stylesheet was requested, and the browser got the nice html error page instead. See the similar bug #14520, https://bugzilla.wikimedia.org/show_bug.cgi?id=14520 It should be easy to fix, either by including blank such files, or adding a more complicated check to wikibits.js: Line 96 of wikibits.js ( http://svn.wikimedia.org/svnroot/mediawiki/trunk/phase3/skins/common/wikibits.js rev 37012 or http://en.wikipedia.org/skins-1.5/common/wikibits.js ) is the following: document.write('<link rel="stylesheet" type="text/css" href="'+stylepath+'/'+skin+'/KHTMLFixes.css">'); I believe this could be changed to: if(skin=='monobook') document.write('<link rel="stylesheet" type="text/css" href="'+stylepath+'/'+skin+'/KHTMLFixes.css">'); since this is the only skin using the file. The missing files from SVN are: skins/chick/KHTMLFixes.css skins/modern/KHTMLFixes.css skins/myskin/KHTMLFixes.css skins/simple/KHTMLFixes.css The missing files from wikipedia are: skins-1.5/chick/KHTMLFixes.css skins-1.5/cologneblue/KHTMLFixes.css skins-1.5/modern/KHTMLFixes.css skins-1.5/myskin/KHTMLFixes.css skins-1.5/nostalgia/KHTMLFixes.css skins-1.5/simple/KHTMLFixes.css skins-1.5/standard/KHTMLFixes.css
Fixed in r37063.
Reverted in r37067.
Created attachment 5157 [details] Patch to wikibits Added a new function in wikibits called "doesFileExist". Given a relative path beyond wgServer, the function will return true if the HTTP request to it returns with status 200. Applied it to importStyleSheetURI, so it will only add a stylesheet if said file exists.
Um, so this adds an extra HTTP request on every page load for every stylesheet, unconditionally? This doesn't strike you as a bad idea? I would suggest that the skin set an extra JS var that can be checked, as Brion suggested in his commit message.
Indeed, this check would do much more harm than good. The browser already does this 404 check for us by... trying to load it... :) It would probably be best if we can dump these extra workaround scripts as much as possible, though, or make the code for them very skin-specific.
Assigned to ^demon, who has already put the most work in this, and is probably the right person to fix it, after adressing comment 4 and comment 5.
I suggest moving all skin-specific JS to /skins/skinname/common.js and *requiring* that file to exist for every skin (albeit possibly empty) so it can just be included unconditionally and forgotten about.
Is there really a need for a requirement for a common.js? If someone creates a skin that needs an additional javascript, then it is a trivial task to include it in the Skinname.php. Right now only monobook requires those additional javscript css includes. so perhaps monobook should just include this javascript itself, and if another skin wants to do so, they can also include it.
It's sloppy to include empty scripts. It increases HTTP requests and latency unnecessarily. Some browsers will block all parsing while waiting for the script to arrive, even though it will turn out to be empty. Includes should be included only if they're actually needed.
Created attachment 5839 [details] Keep it simple Not much need to make it more complicated than it needs to be. If it doesn't exist, don't request it!
Not an issue anymore, as KTHMLFixes was removed in r53141.
Doesn't the javascript still include opera fixes etc, which may not exist on a custom skin.
Created attachment 6953 [details] add skin specific config to choose fixes for wikibits.js to include Earlier patch was rejected by Brion, as it was monobook specific, and didn't address the other css files for other browsers. Here is a patch that has an additional skin variable that contains an associative array where each "buggy browser" can have an associated css fix file with it. I have included this for monobook. I don't think any of the other skin's need these fixes. I also changed the parameter for makeGlobalVariablesScript to be the skin object/class rather than just the name of the skin, so it can get the data about the required css' from the skin to pass to the javascript. I chose this over adding an additional parameter, in case more stuff was needed in the future, but feel free to make it two parameters if preferred,
reopening bug : see previous patch
Created attachment 6954 [details] add skin specific config to choose fixes for wikibits.js to include Sorry. wrong version. here is correct diff
Is there a big advantage here over just hardcoding it into wikibits.js? In practice, it looks like nothing but Monobook (and Chick, which is closely patterned off Monobook) actually uses separate fixes files. Third-party skins can always inject their own CSS fixes through other means if they really want them, or just use CSS selector hacks like Vector does.
well hardcoding it would have been much easier based on skin name, but a previous patch got reverted for this reason by brion, so i made it as was "requested" here. Im happy with any fix. I just want the 404's outta my stats ;-)
Committed a hardcoded check for Monobook in r61023.
Created attachment 6955 [details] alternative fix for fixes css inclusion here is an alternative css fix which hardcodes the skinnames. I removed the typedef checks, as I couldn't see how you could get to this point without skinname/stylepath being defined.
