r52864 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r52863‎ \| r52864 \| r52865 >
Date:	21:49, 7 July 2009
Author:	simetrical
Status:	ok
Tags:
Comment:	Unify MonoBook <head> generation with legacy skins This was less work than I thought it would be. The only possible practical difference in MonoBook should be that some IE fixes are moved down; at a glance, I don't see how this would hurt anything, but if it does then some more hacks can be added to move them back up. A bunch of whitespace was changed in both MonoBook and legacy skins. Legacy skins had some stuff moved around in the <head>, but mainly to better match MonoBook, so they should work better if anything, not worse. dir= was added to the <body> in legacy skins, matching MonoBook. There should be no other practical differences. I would strongly encourage people to port the Modern and Vector skins to at least share the <head>-generation code like this. I'd even more strongly encourage people to totally rewrite the skin system, but that's a bigger job. :)
Modified paths:	/trunk/phase3/includes/OutputPage.php (modified) (history) /trunk/phase3/includes/Skin.php (modified) (history) /trunk/phase3/includes/SkinTemplate.php (modified) (history) /trunk/phase3/skins/MonoBook.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/MonoBook.php
—	—	@@ -64,49 +64,23 @@
65	65	* @access private
66	66	*/
67	67	function execute() {
68		~~- global $wgRequest;~~
	68	+ global $wgRequest, $wgOut, $wgStyleVersion, $wgJsMimeType, $wgStylePath;
69	69	$this->skin = $skin = $this->data['skin'];
70	70	$action = $wgRequest->getText( 'action' );
71	71
72	72	// Suppress warnings to prevent notices about missing indexes in $this->data
73	73	wfSuppressWarnings();
74	74
75		~~-?><!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">~~
76		~~-<html xmlns="<?php $this->text('xhtmldefaultnamespace') ?>" <?php~~
77		~~- foreach($this->data['xhtmlnamespaces'] as $tag => $ns) {~~
78		~~- ?>xmlns:<?php echo "{$tag}=\"{$ns}\" ";~~
79		~~- } ?>xml:lang="<?php $this->text('lang') ?>" lang="<?php $this->text('lang') ?>" dir="<?php $this->text('dir') ?>">~~
80		~~- <head>~~
81		~~- <meta http-equiv="Content-Type" content="<?php $this->text('mimetype') ?>; charset=<?php $this->text('charset') ?>" />~~
82		~~- <?php $this->html('headlinks') ?>~~
83		~~- <title><?php $this->text('pagetitle') ?></title>~~
84		~~- <?php $this->html('csslinks') ?>~~
	75	+ $path = htmlspecialchars( $wgStylePath );
	76	+ $wgOut->addScript( <<<HTML
	77	+<!--[if lt IE 7]><script type="$wgJsMimeType" src="$path/common/IEFixes.js?$wgStyleVersion"></script>
	78	+ <meta http-equiv="imagetoolbar" content="no" /><![endif]-->
	79	+HTML
	80	+ );
85	81
86		~~- <!--[if lt IE 7]><script type="<?php $this->text('jsmimetype') ?>" src="<?php $this->text('stylepath') ?>/common/IEFixes.js?<?php echo $GLOBALS['wgStyleVersion'] ?>"></script>~~
87		~~- <meta http-equiv="imagetoolbar" content="no" /><![endif]-->~~
	82	+ echo $wgOut->headElement( $this->skin );
88	83
89		~~- <?php print Skin::makeGlobalVariablesScript( $this->data ); ?>~~
90		-
91		~~- <script type="<?php $this->text('jsmimetype') ?>" src="<?php $this->text('stylepath' ) ?>/common/wikibits.js?<?php echo $GLOBALS['wgStyleVersion'] ?>"></script>~~
92		~~-<?php $this->html('headscripts') ?>~~
93		~~-<?php if($this->data['jsvarurl']) { ?>~~
94		~~- <script type="<?php $this->text('jsmimetype') ?>" src="<?php $this->text('jsvarurl') ?>"></script>~~
95		~~-<?php } ?>~~
96		~~-<?php if($this->data['pagecss']) { ?>~~
97		~~- <style type="text/css"><?php $this->html('pagecss') ?></style>~~
98		~~-<?php }~~
99		~~- if($this->data['usercss']) { ?>~~
100		~~- <style type="text/css"><?php $this->html('usercss') ?></style>~~
101		~~-<?php }~~
102		~~- if($this->data['userjs']) { ?>~~
103		~~- <script type="<?php $this->text('jsmimetype') ?>" src="<?php $this->text('userjs' ) ?>"></script>~~
104		~~-<?php }~~
105		~~- if($this->data['userjsprev']) { ?>~~
106		~~- <script type="<?php $this->text('jsmimetype') ?>"><?php $this->html('userjsprev') ?></script>~~
107		~~-<?php }~~
108		~~- if($this->data['trackbackhtml']) print $this->data['trackbackhtml']; ?>~~
109		~~- </head>~~
110		~~-<body<?php if($this->data['body_ondblclick']) { ?> ondblclick="<?php $this->text('body_ondblclick') ?>"<?php } ?>~~
	84	+?><body<?php if($this->data['body_ondblclick']) { ?> ondblclick="<?php $this->text('body_ondblclick') ?>"<?php } ?>
111	85	<?php if($this->data['body_onload']) { ?> onload="<?php $this->text('body_onload') ?>"<?php } ?>
112	86	class="mediawiki <?php $this->text('dir'); $this->text('capitalizeallnouns') ?> <?php $this->text('pageclass') ?> <?php $this->text('skinnameclass') ?>">
113	87	<div id="globalWrapper">
Index: trunk/phase3/includes/OutputPage.php
—	—	@@ -93,7 +93,7 @@
94	94	array_push( $this->mKeywords, $text );
95	95	}
96	96	}
97		~~- function addScript( $script ) { $this->mScripts .= "\t\t" . $script . "\n"; }~~
	97	+ function addScript( $script ) { $this->mScripts .= "\t" . $script . "\n"; }
98	98
99	99	function addExtensionStyle( $url ) {
100	100	$linkarr = array( 'rel' => 'stylesheet', 'href' => $url, 'type' => 'text/css' );
—	—	@@ -1548,8 +1548,10 @@
1549	1549	global $wgXhtmlDefaultNamespace, $wgXhtmlNamespaces;
1550	1550	global $wgContLang, $wgUseTrackbacks, $wgStyleVersion;
1551	1551
1552		~~- $this->addMeta( "http:Content-type", "$wgMimeType; charset={$wgOutputEncoding}" );~~
1553		~~- $this->addStyle( 'common/wikiprintable.css', 'print' );~~
	1552	+ $this->addMeta( "http:Content-Type", "$wgMimeType; charset={$wgOutputEncoding}" );
	1553	+ if ( $sk->commonPrintStylesheet() ) {
	1554	+ $this->addStyle( 'common/wikiprintable.css', 'print' );
	1555	+ }
1554	1556	$sk->setupUserCss( $this );
1555	1557
1556	1558	$ret = '';
—	—	@@ -1558,24 +1560,23 @@
1559	1561	$ret .= "<?xml version=\"1.0\" encoding=\"$wgOutputEncoding\" ?" . ">\n";
1560	1562	}
1561	1563
1562		~~- $ret .= "<!DOCTYPE html PUBLIC \"$wgDocType\"\n \"$wgDTD\">\n";~~
	1564	+ $ret .= "<!DOCTYPE html PUBLIC \"$wgDocType\" \"$wgDTD\">\n";
1563	1565
1564	1566	if ( '' == $this->getHTMLTitle() ) {
1565	1567	$this->setHTMLTitle( wfMsg( 'pagetitle', $this->getPageTitle() ));
1566	1568	}
1567	1569
1568		~~- $rtl = $wgContLang->isRTL() ? " dir='RTL'" : '';~~
	1570	+ $dir = $wgContLang->isRTL() ? 'rtl' : 'ltr';
1569	1571	$ret .= "<html xmlns=\"{$wgXhtmlDefaultNamespace}\" ";
1570	1572	foreach($wgXhtmlNamespaces as $tag => $ns) {
1571	1573	$ret .= "xmlns:{$tag}=\"{$ns}\" ";
1572	1574	}
1573		~~- $ret .= "xml:lang=\"$wgContLanguageCode\" lang=\"$wgContLanguageCode\" $rtl>\n";~~
1574		~~- $ret .= "<head>\n<title>" . htmlspecialchars( $this->getHTMLTitle() ) . "</title>\n\t\t";~~
1575		~~- $ret .= implode( "\t\t", array(~~
	1575	+ $ret .= "xml:lang=\"$wgContLanguageCode\" lang=\"$wgContLanguageCode\" dir=\"$dir\">\n";
	1576	+ $ret .= "<head>\n\t<title>" . htmlspecialchars( $this->getHTMLTitle() ) . "</title>\n\t";
	1577	+ $ret .= implode( "\n", array(
1576	1578	$this->getHeadLinks(),
1577	1579	$this->buildCssLinks(),
1578		~~- $sk->getHeadScripts( $this->mAllowUserJs ),~~
1579		~~- $this->mScripts,~~
	1580	+ $sk->getHeadScripts( $this->mAllowUserJs, $this->mScripts ),
1580	1581	$this->getHeadItems(),
1581	1582	));
1582	1583	if( $sk->usercss ){
—	—	@@ -1591,6 +1592,14 @@
1592	1593
1593	1594	protected function addDefaultMeta() {
1594	1595	global $wgVersion;
	1596	+
	1597	+ static $called = false;
	1598	+ if ( $called ) {
	1599	+ # Don't run this twice
	1600	+ return;
	1601	+ }
	1602	+ $called = true;
	1603	+
1595	1604	$this->addMeta( 'http:Content-Style-Type', 'text/css' ); //bug 15835
1596	1605	$this->addMeta( 'generator', "MediaWiki $wgVersion" );
1597	1606
—	—	@@ -1679,7 +1688,7 @@
1680	1689	}
1681	1690	}
1682	1691
1683		~~- return implode( "\n\t\t", $tags ) . "\n";~~
	1692	+ return implode( "\n\t", $tags ) . "\n";
1684	1693	}
1685	1694
1686	1695	/**
—	—	@@ -1746,7 +1755,7 @@
1747	1756	$links[] = $link;
1748	1757	}
1749	1758
1750		~~- return implode( "\n\t\t", $links );~~
	1759	+ return "\t" . implode( "\n\t", $links );
1751	1760	}
1752	1761
1753	1762	protected function styleLink( $style, $options ) {
Index: trunk/phase3/includes/SkinTemplate.php
—	—	@@ -1017,6 +1017,10 @@
1018	1018	wfProfileOut( __METHOD__ );
1019	1019	return $out;
1020	1020	}
	1021	+
	1022	+ public function commonPrintStylesheet() {
	1023	+ return false;
	1024	+ }
1021	1025	}
1022	1026
1023	1027	/**
Index: trunk/phase3/includes/Skin.php
—	—	@@ -447,12 +447,31 @@
448	448	return self::makeVariablesScript( $vars );
449	449	}
450	450
451		~~- function getHeadScripts( $allowUserJs ) {~~
	451	+ /**
	452	+ * Return a random selection of the scripts we want in the header,
	453	+ * according to no particular rhyme or reason. Various other scripts are
	454	+ * returned from a haphazard assortment of other functions scattered over
	455	+ * various files. This entire hackish system needs to be burned to the
	456	+ * ground and rebuilt.
	457	+ *
	458	+ * @var $allowUserJs bool Should probably be identical to $wgAllowUserJs,
	459	+ * but is passed as a local variable for some
	460	+ * obscure reason.
	461	+ * @var $extraHtml string A bunch of raw HTML to jam into some arbitrary
	462	+ * place where MonoBook has historically wanted it.
	463	+ * Old-style skins formerly put it in a different
	464	+ * place, but if either of those is broken it's
	465	+ * likely to be the old-style skins.
	466	+ * @return string Raw HTML to output in some location in the <head> that's
	467	+ * entirely arbitrary but which will probably break
	468	+ * everything if you put it someplace else.
	469	+ */
	470	+ function getHeadScripts( $allowUserJs, $extraHtml = '' ) {
452	471	global $wgStylePath, $wgUser, $wgJsMimeType, $wgStyleVersion;
453	472
454	473	$vars = self::makeGlobalVariablesScript( array( 'skinname' => $this->getSkinName() ) );
455	474
456		~~- $r = array( "<script type=\"{$wgJsMimeType}\" src=\"{$wgStylePath}/common/wikibits.js?$wgStyleVersion\"></script>" );~~
	475	+ $r = array( "<script type=\"{$wgJsMimeType}\" src=\"{$wgStylePath}/common/wikibits.js?$wgStyleVersion\"></script>\n$extraHtml" );
457	476	global $wgUseSiteJs;
458	477	if( $wgUseSiteJs ) {
459	478	$jsCache = $wgUser->isLoggedIn() ? '&smaxage=0' : '';
—	—	@@ -460,7 +479,7 @@
461	480	htmlspecialchars( self::makeUrl( '-',
462	481	"action=raw$jsCache&gen=js&useskin=" .
463	482	urlencode( $this->getSkinName() ) ) ) .
464		~~- "\"><!-- site js --></script>";~~
	483	+ "\"></script>";
465	484	}
466	485	if( $allowUserJs && $wgUser->isLoggedIn() ) {
467	486	$userpage = $wgUser->getUserPage();
—	—	@@ -469,7 +488,7 @@
470	489	'action=raw&ctype='.$wgJsMimeType ) );
471	490	$r[] = '<script type="'.$wgJsMimeType.'" src="'.$userjs."\"></script>";
472	491	}
473		~~- return $vars . "\t\t" . implode ( "\n\t\t", $r );~~
	492	+ return $vars . "\t" . implode ( "\n\t", $r );
474	493	}
475	494
476	495	/**
—	—	@@ -2055,4 +2074,16 @@
2056	2075	wfProfileOut( __METHOD__ );
2057	2076	return $bar;
2058	2077	}
	2078	+
	2079	+ /**
	2080	+ * Should we include common/wikiprintable.css? Skins that have their own
	2081	+ * print stylesheet should override this and return false. (This is an
	2082	+ * ugly hack to get Monobook to play nicely with
	2083	+ * OutputPage::headElement().)
	2084	+ *
	2085	+ * @return bool
	2086	+ */
	2087	+ public function commonPrintStylesheet() {
	2088	+ return true;
	2089	+ }
2059	2090	}

Follow-up revisions

Revision	Commit summary	Author	Date
r52925	fix for r52864: PHP Strict Standards: Declaration of SkinStandard::getHeadSc...	ialex	17:09, 8 July 2009
r53029	Finish cross-skin unification of <head>...	simetrical	02:13, 10 July 2009

Status & tagging log

16:47, 27 October 2009 Catrope (talk | contribs) changed the tags for r52864 [removed: acaifix]
19:22, 22 August 2009 Brion VIBBER (talk | contribs) changed the status of r52864 [removed: new added: ok]
20:25, 21 July 2009 Trevor Parscal (WMF) (talk | contribs) changed the tags for r52864 [added: acaifix]