r38116 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r38115‎ | r38116 | r38117 >
Date:05:09, 28 July 2008
Author:brion
Status:old
Tags:
Comment:
Start on some cleanup of how CSS stylesheets are loaded. Initially addressing only the SkinTemplate-based skins; would like to rip out some near-dupe code in the other skin types, with a little more refactoring...

* A skin can make calls to $this->addScript to much more cleanly list which style sheets it wants to load, for which media variants and which IE conditional versions. This replaces the 'cssfiles' array hack and giant pile of ugly conditionals in MonoBook's template.

* 'printable=yes' and 'handheld=yes' URL options are handled transparently -- 'screen' stylesheets are hidden, while those with no media specifier are left.

MediaWiki:Common.css is now listed without media -- so infoboxes are still formatted -- while the skin-specific eg MediaWiki:Monobook.css are listed for screen, as they're specific to the on-screen skin.

Note it should be a matter of one line of code to add a MediaWiki:Print.css and have it correctly handled now.

* All sheets are now loaded via <link rel="stylesheet"> instead of a mix of those and @import decls.

IIRC we had used @import originally to hide styles from Netscape 4, which tends to utterly break on MonoBook, but these days that's pretty much a non-issue.
@import also breaks some browsers' ability to save stylesheets with a file to disk, which sucks.

Confirmed that Firefox 3 can now save pages with their styles.

* 'screen, projection' media specifier has been changed to just 'screen' -- projection is something totally different.

* Added experimental options for specifying handheld stylesheets:

/**
* Optionally, we can specify a stylesheet to use for media="handheld".
* This is recognized by some, but not all, handheld/mobile/PDA browsers.
* If left empty, compliant handheld browsers won't pick up the skin
* stylesheet, which is specified for 'screen' media.
*
* Can be a complete URL, base-relative path, or $wgStylePath-relative path.
* Try 'chick/main.css' to apply the Chick styles to the MonoBook HTML.
*
* Will also be switched in when 'handheld=yes' is added to the URL, like
* the 'printable=yes' mode for print media.
*/
$wgHandheldStyle = false;

/**
* If set, 'screen' and 'handheld' media specifiers for stylesheets are
* transformed such that they apply to the iPhone/iPod Touch Mobile Safari,
* which doesn't recognize 'handheld' but does support media queries on its
* screen size.
*
* Consider only using this if you have a *really good* handheld stylesheet,
* as iPhone users won't have any way to disable it and use the "grown-up"
* styles instead.
*/
$wgHandheldForIPhone = false;
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/skins/Chick.php (modified) (history)
  • /trunk/phase3/skins/MonoBook.php (modified) (history)
  • /trunk/phase3/skins/chick/main.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/chick/main.css
@@ -419,7 +419,7 @@
420420 padding: 2px 4px;
421421 }
422422
423 -#jump-to-nav {
 423+#xjump-to-nav {
424424 display: none;
425425 }
426426
@@ -430,3 +430,21 @@
431431 div#mw-recreate-deleted-warn ul li {
432432 font-size: 95%;
433433 }
 434+
 435+
 436+.printfooter {
 437+ display: none;
 438+}
 439+
 440+#footer {
 441+ background-color: white;
 442+ border-top: 1px solid #fabd23;
 443+ border-bottom: 1px solid #fabd23;
 444+ margin: .6em 0 1em 0;
 445+ padding: .4em 0 1.2em 0;
 446+ text-align: center;
 447+ font-size: 90%;
 448+}
 449+#f-poweredbyico, #f-copyrightico {
 450+ display: inline;
 451+}
\ No newline at end of file
Index: trunk/phase3/skins/Chick.php
@@ -23,7 +23,12 @@
2424 $this->skinname = 'chick';
2525 $this->stylename = 'chick';
2626 $this->template = 'MonoBookTemplate';
27 - $this->fixfiles = array( 'IE50', 'IE55', 'IE60' );
 27+
 28+ // Append to the default screen common & print styles...
 29+ $this->addStyle( 'chick/main.css', 'screen,handheld' );
 30+ $this->addStyle( 'chick/IE50Fixes.css', 'screen,handheld', 'lt IE 5.5000' );
 31+ $this->addStyle( 'chick/IE55Fixes.css', 'screen,handheld', 'IE 5.5000' );
 32+ $this->addStyle( 'chick/IE60Fixes.css', 'screen,handheld', 'IE 6' );
2833 }
2934 }
3035
Index: trunk/phase3/skins/MonoBook.php
@@ -21,13 +21,26 @@
2222 class SkinMonoBook extends SkinTemplate {
2323 /** Using monobook. */
2424 function initPage( &$out ) {
 25+ global $wgHandheldStyle;
 26+
2527 SkinTemplate::initPage( $out );
2628 $this->skinname = 'monobook';
2729 $this->stylename = 'monobook';
2830 $this->template = 'MonoBookTemplate';
29 - # Bug 14520: skins that just include this file shouldn't load nonexis-
30 - # tent CSS fix files.
31 - $this->cssfiles = array( 'IE', 'IE50', 'IE55', 'IE60', 'IE70', 'rtl' );
 31+
 32+ // Append to the default screen common & print styles...
 33+ $this->addStyle( 'monobook/main.css', 'screen' );
 34+ if( $wgHandheldStyle ) {
 35+ // Currently in testing... try 'chick/main.css'
 36+ $this->addStyle( $wgHandheldStyle, 'handheld' );
 37+ }
 38+
 39+ $this->addStyle( 'monobook/IE50Fixes.css', 'screen', 'lt IE 5.5000' );
 40+ $this->addStyle( 'monobook/IE55Fixes.css', 'screen', 'IE 5.5000' );
 41+ $this->addStyle( 'monobook/IE60Fixes.css', 'screen', 'IE 6' );
 42+ $this->addStyle( 'monobook/IE70Fixes.css', 'screen', 'IE 7' );
 43+
 44+ $this->addStyle( 'monobook/rtl.css', 'screen', '', 'rtl' );
3245 }
3346 }
3447
@@ -62,17 +75,10 @@
6376 <meta http-equiv="Content-Type" content="<?php $this->text('mimetype') ?>; charset=<?php $this->text('charset') ?>" />
6477 <?php $this->html('headlinks') ?>
6578 <title><?php $this->text('pagetitle') ?></title>
66 - <style type="text/css" media="screen, projection">/*<![CDATA[*/
67 - @import "<?php $this->text('stylepath') ?>/common/shared.css?<?php echo $GLOBALS['wgStyleVersion'] ?>";
68 - @import "<?php $this->text('stylepath') ?>/<?php $this->text('stylename') ?>/main.css?<?php echo $GLOBALS['wgStyleVersion'] ?>";
69 - /*]]>*/</style>
70 - <link rel="stylesheet" type="text/css" <?php if(empty($this->data['printable']) ) { ?>media="print"<?php } ?> href="<?php $this->text('printcss') ?>?<?php echo $GLOBALS['wgStyleVersion'] ?>" />
71 - <?php if( in_array( 'IE50', $skin->cssfiles ) ) { ?><!--[if lt IE 5.5000]><style type="text/css">@import "<?php $this->text('stylepath') ?>/<?php $this->text('stylename') ?>/IE50Fixes.css?<?php echo $GLOBALS['wgStyleVersion'] ?>";</style><![endif]-->
72 - <?php } if( in_array( 'IE55', $skin->cssfiles ) ) { ?><!--[if IE 5.5000]><style type="text/css">@import "<?php $this->text('stylepath') ?>/<?php $this->text('stylename') ?>/IE55Fixes.css?<?php echo $GLOBALS['wgStyleVersion'] ?>";</style><![endif]-->
73 - <?php } if( in_array( 'IE60', $skin->cssfiles ) ) { ?><!--[if IE 6]><style type="text/css">@import "<?php $this->text('stylepath') ?>/<?php $this->text('stylename') ?>/IE60Fixes.css?<?php echo $GLOBALS['wgStyleVersion'] ?>";</style><![endif]-->
74 - <?php } if( in_array( 'IE70', $skin->cssfiles ) ) { ?><!--[if IE 7]><style type="text/css">@import "<?php $this->text('stylepath') ?>/<?php $this->text('stylename') ?>/IE70Fixes.css?<?php echo $GLOBALS['wgStyleVersion'] ?>";</style><![endif]-->
75 - <?php } ?><!--[if lt IE 7]><?php if( in_array( 'IE', $skin->cssfiles ) ) { ?><script type="<?php $this->text('jsmimetype') ?>" src="<?php $this->text('stylepath') ?>/common/IEFixes.js?<?php echo $GLOBALS['wgStyleVersion'] ?>"></script>
76 - <?php } ?><meta http-equiv="imagetoolbar" content="no" /><![endif]-->
 79+<?php $this->html('csslinks') ?>
 80+
 81+ <!--[if lt IE 7]><script type="<?php $this->text('jsmimetype') ?>" src="<?php $this->text('stylepath') ?>/common/IEFixes.js?<?php echo $GLOBALS['wgStyleVersion'] ?>"></script>
 82+ <meta http-equiv="imagetoolbar" content="no" /><![endif]-->
7783
7884 <?php print Skin::makeGlobalVariablesScript( $this->data ); ?>
7985
Index: trunk/phase3/includes/SkinTemplate.php
@@ -86,12 +86,13 @@
8787 * will actually fill the template.
8888 */
8989 var $template;
90 -
 90+
9191 /**
92 - * An array of strings representing extra CSS files to load. May include:
93 - * 'IE', 'IE50', 'IE55', 'IE60', 'IE70', 'rtl'.
 92+ * An array of stylesheet filenames (relative from skins path), with options
 93+ * for CSS media, IE conditions, and RTL/LTR direction.
 94+ * For internal use; add settings in the skin via $this->addStyle()
9495 */
95 - var $cssfiles;
 96+ var $styles = array();
9697
9798 /**#@-*/
9899
@@ -107,7 +108,9 @@
108109 $this->skinname = 'monobook';
109110 $this->stylename = 'monobook';
110111 $this->template = 'QuickTemplate';
111 - $this->cssfiles = array();
 112+
 113+ $this->addStyle( 'common/shared.css', 'screen' );
 114+ $this->addStyle( 'common/commonPrint.css', 'print' );
112115 }
113116
114117 /**
@@ -247,6 +250,8 @@
248251 $tpl->set( 'skinclass', get_class( $this ) );
249252 $tpl->setRef( 'stylename', $this->stylename );
250253 $tpl->set( 'printable', $wgRequest->getBool( 'printable' ) );
 254+ $tpl->set( 'handheld', $wgRequest->getBool( 'handheld' ) );
 255+ $tpl->set( 'csslinks', $this->buildCssLinks() );
251256 $tpl->setRef( 'loggedin', $this->loggedin );
252257 $tpl->set('nsclass', 'ns-'.$this->mTitle->getNamespace());
253258 $tpl->set('notspecialpage', $this->mTitle->getNamespace() != NS_SPECIAL);
@@ -274,7 +279,6 @@
275280 $tpl->setRef( 'userpageurl', $this->userpageUrlDetails['href']);
276281 $tpl->set( 'userlang', $wgLang->getCode() );
277282 $tpl->set( 'pagecss', $this->setupPageCss() );
278 - $tpl->set( 'printcss', $this->getPrintCss() );
279283 $tpl->setRef( 'usercss', $this->usercss);
280284 $tpl->setRef( 'userjs', $this->userjs);
281285 $tpl->setRef( 'userjsprev', $this->userjsprev);
@@ -964,54 +968,49 @@
965969
966970 global $wgRequest, $wgAllowUserCss, $wgUseSiteCss, $wgContLang, $wgSquidMaxage, $wgStylePath, $wgUser;
967971
968 - $sitecss = '';
969972 $usercss = '';
970973 $siteargs = '&maxage=' . $wgSquidMaxage;
971974 if( $this->loggedin ) {
972975 // Ensure that logged-in users' generated CSS isn't clobbered
973976 // by anons' publicly cacheable generated CSS.
974977 $siteargs .= '&smaxage=0';
975 - }
976 -
977 - # Add user-specific code if this is a user and we allow that kind of thing
978 -
979 - if ( $wgAllowUserCss && $this->loggedin ) {
980 - $action = $wgRequest->getText('action');
981 -
982 - # if we're previewing the CSS page, use it
983 - if( $this->mTitle->isCssSubpage() and $this->userCanPreview( $action ) ) {
984 - $siteargs = "&smaxage=0&maxage=0";
985 - $usercss = $wgRequest->getText('wpTextbox1');
986 - } else {
987 - $usercss = '@import "' .
988 - self::makeUrl($this->userpage . '/'.$this->skinname.'.css',
989 - 'action=raw&ctype=text/css') . '";' ."\n";
990 - }
991 -
992978 $siteargs .= '&ts=' . $wgUser->mTouched;
993979 }
994980
995 - if( $wgContLang->isRTL() && in_array( 'rtl', $this->cssfiles ) ) {
996 - global $wgStyleVersion;
997 - $sitecss .= "@import \"$wgStylePath/$this->stylename/rtl.css?$wgStyleVersion\";\n";
998 - }
999 -
1000981 # If we use the site's dynamic CSS, throw that in, too
 982+ // Per-site custom styles
1001983 if ( $wgUseSiteCss ) {
1002984 $query = "usemsgcache=yes&action=raw&ctype=text/css&smaxage=$wgSquidMaxage";
1003985 $skinquery = '';
1004986 if (($us = $wgRequest->getVal('useskin', '')) !== '')
1005987 $skinquery = "&useskin=$us";
1006 - $sitecss .= '@import "' . self::makeNSUrl( 'Common.css', $query, NS_MEDIAWIKI) . '";' . "\n";
1007 - $sitecss .= '@import "' . self::makeNSUrl( ucfirst( $this->skinname ) . '.css', $query, NS_MEDIAWIKI ) . '";' . "\n";
1008 - $sitecss .= '@import "' . self::makeUrl( '-', "action=raw&gen=css$siteargs$skinquery" ) . '";' . "\n";
 988+
 989+ $this->addStyle( self::makeNSUrl( 'Common.css', $query, NS_MEDIAWIKI) );
 990+ $this->addStyle( self::makeNSUrl( ucfirst( $this->skinname ) . '.css', $query, NS_MEDIAWIKI ),
 991+ 'screen' );
1009992 }
1010993
1011 - # If we use any dynamic CSS, make a little CDATA block out of it.
 994+ // Per-user styles based on preferences
 995+ $this->addStyle( self::makeUrl( '-', "action=raw&gen=css$siteargs$skinquery" ), 'screen' );
1012996
1013 - if ( !empty($sitecss) || !empty($usercss) ) {
1014 - $this->usercss = "/*<![CDATA[*/\n" . $sitecss . $usercss . '/*]]>*/';
 997+ // Per-user custom style pages
 998+ if ( $wgAllowUserCss && $this->loggedin ) {
 999+ $action = $wgRequest->getVal('action');
 1000+
 1001+ # if we're previewing the CSS page, use it
 1002+ if( $this->mTitle->isCssSubpage() and $this->userCanPreview( $action ) ) {
 1003+ $previewCss = $wgRequest->getText('wpTextbox1');
 1004+
 1005+ /// @fixme properly escape the cdata!
 1006+ $this->usercss = "/*<![CDATA[*/\n" .
 1007+ $previewCss .
 1008+ "/*]]>*/";
 1009+ } else {
 1010+ $this->addStyle( self::makeUrl($this->userpage . '/'.$this->skinname.'.css',
 1011+ 'action=raw&ctype=text/css'), 'screen' );
 1012+ }
10151013 }
 1014+
10161015 wfProfileOut( __METHOD__ );
10171016 }
10181017
@@ -1063,17 +1062,6 @@
10641063 }
10651064
10661065 /**
1067 - * Returns the print stylesheet for this skin. In all default skins this
1068 - * is just commonPrint.css, but third-party skins may want to modify it.
1069 - *
1070 - * @return string
1071 - */
1072 - protected function getPrintCss() {
1073 - global $wgStylePath;
1074 - return $wgStylePath . "/common/commonPrint.css";
1075 - }
1076 -
1077 - /**
10781066 * This returns MediaWiki:Common.js and MediaWiki:[Skinname].js concate-
10791067 * nated together. For some bizarre reason, it does *not* return any
10801068 * custom user JS from subpages. Huh?
@@ -1102,6 +1090,118 @@
11031091 wfProfileOut( __METHOD__ );
11041092 return $s;
11051093 }
 1094+
 1095+ /**
 1096+ * Add a local or specified stylesheet, with the given media options.
 1097+ * Meant primarily for internal use...
 1098+ *
 1099+ * @param $media -- to specify a media type, 'screen', 'printable', 'handheld' or any.
 1100+ * @param $conditional -- for IE conditional comments, specifying an IE version
 1101+ * @param $dir -- set to 'rtl' or 'ltr' for direction-specific sheets
 1102+ */
 1103+ public function addStyle( $style, $media='', $condition='', $dir='' ) {
 1104+ $options = array();
 1105+ if( $media )
 1106+ $options['media'] = $media;
 1107+ if( $condition )
 1108+ $options['condition'] = $condition;
 1109+ if( $dir )
 1110+ $options['dir'] = $dir;
 1111+ $this->styles[$style] = $options;
 1112+ }
 1113+
 1114+ /**
 1115+ * Build a set of <link>s for the stylesheets specified in the $this->styles array.
 1116+ * These will be applied to various media & IE conditionals.
 1117+ */
 1118+ protected function buildCssLinks() {
 1119+ global $wgContLang;
 1120+
 1121+ foreach( $this->styles as $file => $options ) {
 1122+ $links[] = $this->styleLink( $file, $options );
 1123+ }
 1124+
 1125+ return implode( "\n", $links );
 1126+ }
 1127+
 1128+ protected function styleLink( $style, $options ) {
 1129+ global $wgRequest;
 1130+
 1131+ if( isset( $options['dir'] ) ) {
 1132+ global $wgContLang;
 1133+ $siteDir = $wgContLang->isRTL() ? 'rtl' : 'ltr';
 1134+ if( $siteDir != $options['dir'] )
 1135+ return '';
 1136+ }
 1137+
 1138+ if( isset( $options['media'] ) ) {
 1139+ $media = $this->transformCssMedia( $options['media'] );
 1140+ if( is_null( $media ) ) {
 1141+ return '';
 1142+ }
 1143+ } else {
 1144+ $media = '';
 1145+ }
 1146+
 1147+ if( substr( $style, 0, 1 ) == '/' ||
 1148+ substr( $style, 0, 5 ) == 'http:' ||
 1149+ substr( $style, 0, 6 ) == 'https:' ) {
 1150+ $url = $style;
 1151+ } else {
 1152+ global $wgStylePath, $wgStyleVersion;
 1153+ $url = $wgStylePath . '/' . $style . '?' . $wgStyleVersion;
 1154+ }
 1155+
 1156+ $attribs = array(
 1157+ 'rel' => 'stylesheet',
 1158+ 'href' => $url,
 1159+ 'type' => 'text/css' );
 1160+ if( $media ) {
 1161+ $attribs['media'] = $media;
 1162+ }
 1163+
 1164+ $link = Xml::element( 'link', $attribs );
 1165+
 1166+ if( isset( $options['condition'] ) ) {
 1167+ $condition = htmlspecialchars( $options['condition'] );
 1168+ $link = "<!--[if $condition]>$link<![endif]-->";
 1169+ }
 1170+ return $link;
 1171+ }
 1172+
 1173+ function transformCssMedia( $media ) {
 1174+ global $wgRequest, $wgHandheldForIPhone;
 1175+
 1176+ // Switch in on-screen display for media testing
 1177+ $switches = array(
 1178+ 'printable' => 'print',
 1179+ 'handheld' => 'handheld',
 1180+ );
 1181+ foreach( $switches as $switch => $targetMedia ) {
 1182+ if( $wgRequest->getBool( $switch ) ) {
 1183+ if( $media == $targetMedia ) {
 1184+ $media = '';
 1185+ } elseif( $media == 'screen' ) {
 1186+ return null;
 1187+ }
 1188+ }
 1189+ }
 1190+
 1191+ // Expand longer media queries as iPhone doesn't grok 'handheld'
 1192+ if( $wgHandheldForIPhone ) {
 1193+ $mediaAliases = array(
 1194+ 'screen' => 'screen and (min-device-width: 481px)',
 1195+ 'handheld' => 'handheld, only screen and (max-device-width: 480px)',
 1196+ );
 1197+
 1198+ if( isset( $mediaAliases[$media] ) ) {
 1199+ $media = $mediaAliases[$media];
 1200+ }
 1201+ }
 1202+
 1203+ return $media;
 1204+ }
 1205+
11061206 }
11071207
11081208 /**
Index: trunk/phase3/includes/DefaultSettings.php
@@ -2137,6 +2137,32 @@
21382138 $wgDefaultSkin = 'monobook';
21392139
21402140 /**
 2141+ * Optionally, we can specify a stylesheet to use for media="handheld".
 2142+ * This is recognized by some, but not all, handheld/mobile/PDA browsers.
 2143+ * If left empty, compliant handheld browsers won't pick up the skin
 2144+ * stylesheet, which is specified for 'screen' media.
 2145+ *
 2146+ * Can be a complete URL, base-relative path, or $wgStylePath-relative path.
 2147+ * Try 'chick/main.css' to apply the Chick styles to the MonoBook HTML.
 2148+ *
 2149+ * Will also be switched in when 'handheld=yes' is added to the URL, like
 2150+ * the 'printable=yes' mode for print media.
 2151+ */
 2152+$wgHandheldStyle = false;
 2153+
 2154+/**
 2155+ * If set, 'screen' and 'handheld' media specifiers for stylesheets are
 2156+ * transformed such that they apply to the iPhone/iPod Touch Mobile Safari,
 2157+ * which doesn't recognize 'handheld' but does support media queries on its
 2158+ * screen size.
 2159+ *
 2160+ * Consider only using this if you have a *really good* handheld stylesheet,
 2161+ * as iPhone users won't have any way to disable it and use the "grown-up"
 2162+ * styles instead.
 2163+ */
 2164+$wgHandheldForIPhone = false;
 2165+
 2166+/**
21412167 * Settings added to this array will override the default globals for the user
21422168 * preferences used by anonymous visitors and newly created accounts.
21432169 * For instance, to disable section editing links:

Follow-up revisions

RevisionCommit summaryAuthorDate
r38139Some tweaks for r38116:...ialex16:54, 28 July 2008
r39021Tweak css load order (broken for extensions since r38116)aaron18:24, 9 August 2008
r39040Revert r39021 "Tweak css load order (broken for extensions since r38116)"...brion00:54, 10 August 2008

Status & tagging log