r74966 MediaWiki - Code Review archive

Clearly I meant to say "Port Monobook to ResourceLoader"

From Translatewiki:

PHP Warning:  filemtime() [<a href='function.filemtime'>function.filemtime</a>]: stat failed for /www/w/Array in /www/w/includes/ResourceLoaderModule.php on line 565

From Translatewiki too:

PHP Warning: file_get_contents(/www/w/Array) [<a href='function.file-get-contents'>function.file-get-contents</a>]: failed to open stream: No such file or directory in /www/w/includes/ResourceLoaderModule.php on line 734

+'skins/monobook/main.css' => array( 'media' => 'screen' ),

+// Honor $wgHandheldStyle. This is kind of evil
+$GLOBALS['wgHandheldStyle'] => array( 'media' => 'handheld' )

The last line makes no sense at all and seems to be the cause for problems mentioned above.

It makes sense: $wgHandheldStyle is supposed to be a path to a file.

However, it's also allowed to be false, in which case the index is 0 and the RL sees a numerically-indexed array, therefore thinking the value (which is an array) is the file name :(

Another report: [[1]].

No concrete report there other than "I changed something in LocalSettings.php and now it works", that's of zero use to me. The only issue mentioned here has been fixed, so resetting to new.

r75006 wasn't a proper fix. $wgHandheldStyle url is no longer loaded.

(Although it still produces the undocumented feature of loading Mediawiki:Handheld.css)

Fixed in r75548.

This totally messes up IE6 for me. The content area takes up the full width of the screen and the sidebar is pushed to the bottom.

Presumably this is because the IE-specific scripts are no longer loaded?

IEFixes*.css, yes. I was hoping Trevor would port those fixes into the main CSS file but that never happened.

This is new:

html > body .portlet {
	float: left;
	clear: left;
}
/* recover IEMac (might be fine with the float, but usually it's close to IE */
*>body .portlet {
	float: none;
	clear: none;
}

it seems to come from rtl.css, but it isn't .rtl noflip atm. I'm also not entirely sure what this rtl specific element was supposed to fix in rtl.

Just for the record, IEMac (or rather "IE5 for Mac") is no longer supported, so this may not be needed anymore if it's causing problems.

All it appears to do is add a lot of spacing in IE 7 and IE 8/9 in IE 7 compatibility mode: bug 31993

If there's no other reason for it, I say we kill it.

Checked with Roan about it on IRC; he doesn't remember much specific details about it but can confirm it was ported in from Monobook's old rtl.css. The floats no longer seem necessary either for LTR or RTL, and removing them (and the IE/Mac fixlet after it) seems fine on everything I've tested so far.

Removed these bits in r101540.

moving to new so krinkle can review to make sure everything has been addressed.

This still has lots of issues, it should just be backed out until we can dedicate some resources to properly reproducing the monobook style in more modern code.

Should be much better with r97623 and r97634. I've only tested in IE8 compat mode, though, so IE5/6 might still be broken, but reinstating the fixes CSS files means that most issues should be resolved at the expense of an additional request.

IE5 should not be an issue imho as it's no longer supported. The content should visible and readable (which it is).

IE6 and IE7 still need attention though.

IE7 (or at least IE8 in IE7 compat mode) looks fine now, or at least not more broken than in 1.17. IE6 seems fine in Reedy's tests other than that the navigation tabs (article/discussion/edit/...) are missing somehow.

Trevor fixed more issues in r97661. Trevor, can this rev be marked resolved now?

Attempted to back this out of 1.18, but CSS merge conflicts. Would you mind doing it Roan?

Yes