Fixed in r37063.

Reverted in r37067.

Created attachment 5157
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.

attachment wikibits.patch ignored as obsolete

ayg wrote:

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.

buzz wrote:

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.

ayg wrote:

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.

bluehairedlawyer wrote:

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!

attachment diff.txt ignored as obsolete

Not an issue anymore, as KTHMLFixes was removed in r53141.

buzz wrote:

Doesn't the javascript still include opera fixes etc, which may not exist on a custom skin.

buzz wrote:

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,

attachment cssfixes.diff ignored as obsolete

buzz wrote:

reopening bug : see previous patch

buzz wrote:

add skin specific config to choose fixes for wikibits.js to include

Sorry. wrong version. here is correct diff

Attached:

ayg wrote:

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.

buzz wrote:

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 ;-)

ayg wrote:

Committed a hardcoded check for Monobook in r61023.

buzz wrote:

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.

Attached: