r56770 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r56769‎ \| r56770 \| r56771 >
Date:	16:52, 22 September 2009
Author:	ialex
Status:	ok
Tags:
Comment:	* (bug 20631) Preview of personal JavaScript and CSS pages now works again * introduced SkinTemplate::$useHeadElement as switch for backward compatibility for extension skins using the old way of generating the <head> element: false (default): no change from previous version true: <head> specific items set in SkinTemplate::outputPage() are no longer generated (avoid double execution of some functions) and the result of OutputPage::headElement() is stored in the 'headelement' item * updated all core skin to use this new method, some extensions using MonoBookTemplate but not extending SkinMonoBook (or for other core skins) will need to set $useHeadElement to true to work properly though * Made Skin::userCanPreview() public since it's needed in OutputPage::getHeadScripts() * Pass the Skin object from OutputPage::headElement() to OutputPage::getHeadScripts() rather than getting it from $wgUser
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/Chick.php (modified) (history) /trunk/phase3/skins/Modern.php (modified) (history) /trunk/phase3/skins/MonoBook.php (modified) (history) /trunk/phase3/skins/MySkin.php (modified) (history) /trunk/phase3/skins/Simple.php (modified) (history) /trunk/phase3/skins/Vector.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/Simple.php
—	—	@@ -19,7 +19,7 @@
20	20	*/
21	21	class SkinSimple extends SkinTemplate {
22	22	var $skinname = 'simple', $stylename = 'simple',
23		~~- $template = 'MonoBookTemplate';~~
	23	+ $template = 'MonoBookTemplate', $useHeadElement = true;
24	24
25	25	function setupSkinUserCss( OutputPage $out ){
26	26	$out->addStyle( 'simple/main.css', 'screen' );
Index: trunk/phase3/skins/Vector.php
—	—	@@ -19,7 +19,7 @@
20	20
21	21	/* Functions */
22	22	var $skinname = 'vector', $stylename = 'vector',
23		~~- $template = 'VectorTemplate';~~
	23	+ $template = 'VectorTemplate', $useHeadElement = true;
24	24
25	25	/**
26	26	* Initializes output page and sets up skin-specific parameters
—	—	@@ -459,7 +459,7 @@
460	460	array_reverse( $this->data['personal_urls'] );
461	461	}
462	462	// Output HTML Page
463		~~- echo $wgOut->headElement( $this->skin );~~
	463	+ $this->html( 'headelement' );
464	464	?>
465	465	<body<?php if ( $this->data['body_ondblclick'] ): ?> ondblclick="<?php $this->text( 'body_ondblclick' ) ?>"<?php endif; ?> <?php if ( $this->data['body_onload'] ): ?> onload="<?php $this->text( 'body_onload' ) ?>"<?php endif; ?> class="mediawiki <?php $this->text( 'dir' ) ?> <?php $this->text( 'pageclass' ) ?> <?php $this->text( 'skinnameclass' ) ?>" dir="<?php $this->text( 'dir' ) ?>">
466	466	<div id="page-base" class="noprint"></div>
Index: trunk/phase3/skins/MySkin.php
—	—	@@ -16,5 +16,5 @@
17	17	*/
18	18	class SkinMySkin extends SkinTemplate {
19	19	var $skinname = 'myskin', $stylename = 'myskin',
20		~~- $template = 'MonoBookTemplate';~~
	20	+ $template = 'MonoBookTemplate', $useHeadElement = true;
21	21	}
Index: trunk/phase3/skins/Chick.php
—	—	@@ -18,12 +18,8 @@
19	19	* @ingroup Skins
20	20	*/
21	21	class SkinChick extends SkinTemplate {
22		~~- function initPage( OutputPage $out ) {~~
23		~~- parent::initPage( $out );~~
24		~~- $this->skinname = 'chick';~~
25		~~- $this->stylename = 'chick';~~
26		~~- $this->template = 'MonoBookTemplate';~~
27		~~- }~~
	22	+ var $skinname = 'chick', $stylename = 'chick',
	23	+ $template = 'MonoBookTemplate', $useHeadElement = true;
28	24
29	25	function setupSkinUserCss( OutputPage $out ){
30	26	parent::setupSkinUserCss( $out );
Index: trunk/phase3/skins/MonoBook.php
—	—	@@ -21,10 +21,10 @@
22	22	class SkinMonoBook extends SkinTemplate {
23	23	/** Using monobook. */
24	24	var $skinname = 'monobook', $stylename = 'monobook',
25		~~- $template = 'MonoBookTemplate';~~
	25	+ $template = 'MonoBookTemplate', $useHeadElement = true;
26	26
27	27	function setupSkinUserCss( OutputPage $out ) {
28		~~- global $wgHandheldStyle;~~
	28	+ global $wgHandheldStyle, $wgStyleVersion, $wgJsMimeType, $wgStylePath;
29	29
30	30	parent::setupSkinUserCss( $out );
31	31
—	—	@@ -42,14 +42,13 @@
43	43
44	44	$out->addStyle( 'monobook/rtl.css', 'screen', '', 'rtl' );
45	45
46		-
47		~~- // @todo We can move this to the parent once we update all the skins~~
48		~~- if( isset( $this->pagecss ) && $this->pagecss )~~
49		~~- $out->addInlineStyle( $this->pagecss );~~
50		-
51		~~- if( isset( $this->usercss ) && $this->usercss )~~
52		~~- $out->addInlineStyle( $this->usercss );~~
53		-
	46	+ # FIXME: What is this? Should it apply to all skins?
	47	+ $path = htmlspecialchars( $wgStylePath );
	48	+ $out->addScript( <<<HTML
	49	+<!--[if lt IE 7]><script type="$wgJsMimeType" src="$path/common/IEFixes.js?$wgStyleVersion"></script>
	50	+ <meta http-equiv="imagetoolbar" content="no" /><![endif]-->
	51	+HTML
	52	+ );
54	53	}
55	54	}
56	55
—	—	@@ -68,23 +67,15 @@
69	68	* @access private
70	69	*/
71	70	function execute() {
72		~~- global $wgRequest, $wgOut, $wgStyleVersion, $wgJsMimeType, $wgStylePath;~~
	71	+ global $wgRequest;
	72	+
73	73	$this->skin = $skin = $this->data['skin'];
74	74	$action = $wgRequest->getText( 'action' );
75	75
76	76	// Suppress warnings to prevent notices about missing indexes in $this->data
77	77	wfSuppressWarnings();
78	78
79		~~- # FIXME: What is this? Should it apply to all skins?~~
80		~~- $path = htmlspecialchars( $wgStylePath );~~
81		~~- $wgOut->addScript( <<<HTML~~
82		~~-<!--[if lt IE 7]><script type="$wgJsMimeType" src="$path/common/IEFixes.js?$wgStyleVersion"></script>~~
83		~~- <meta http-equiv="imagetoolbar" content="no" /><![endif]-->~~
84		~~-HTML~~
85		~~- );~~
86		-
87		~~- echo $wgOut->headElement( $this->skin );~~
88		-
	79	+ $this->html( 'headelement' );
89	80	?><body<?php if($this->data['body_ondblclick']) { ?> ondblclick="<?php $this->text('body_ondblclick') ?>"<?php } ?>
90	81	<?php if($this->data['body_onload']) { ?> onload="<?php $this->text('body_onload') ?>"<?php } ?>
91	82	class="mediawiki <?php $this->text('dir'); $this->text('capitalizeallnouns') ?> <?php $this->text('pageclass') ?> <?php $this->text('skinnameclass') ?>">
Index: trunk/phase3/skins/Modern.php
—	—	@@ -17,7 +17,7 @@
18	18	*/
19	19	class SkinModern extends SkinTemplate {
20	20	var $skinname = 'modern', $stylename = 'modern',
21		~~- $template = 'ModernTemplate';~~
	21	+ $template = 'ModernTemplate', $useHeadElement = true;
22	22
23	23	/*
24	24	* We don't like the default getPoweredBy, the icon clashes with the
—	—	@@ -29,11 +29,20 @@
30	30	}
31	31
32	32	function setupSkinUserCss( OutputPage $out ){
	33	+ global $wgStyleVersion, $wgJsMimeType, $wgStylePath;
	34	+
33	35	// Do not call parent::setupSkinUserCss(), we have our own print style
34	36	$out->addStyle( 'common/shared.css', 'screen' );
35	37	$out->addStyle( 'modern/main.css', 'screen' );
36	38	$out->addStyle( 'modern/print.css', 'print' );
37	39	$out->addStyle( 'modern/rtl.css', 'screen', '', 'rtl' );
	40	+
	41	+ $path = htmlspecialchars( $wgStylePath );
	42	+ $out->addScript( <<<HTML
	43	+<!--[if lt IE 7]><script type="$wgJsMimeType" src="$path/common/IEFixes.js?$wgStyleVersion"></script>
	44	+ <meta http-equiv="imagetoolbar" content="no" /><![endif]-->
	45	+HTML
	46	+ );
38	47	}
39	48	}
40	49
—	—	@@ -59,13 +68,7 @@
60	69	// Suppress warnings to prevent notices about missing indexes in $this->data
61	70	wfSuppressWarnings();
62	71
63		~~- $wgOut->addScript( <<<HTML~~
64		~~-<!--[if lt IE 7]><script type="$wgJsMimeType" src="$path/common/IEFixes.js?$wgStyleVersion"></script>~~
65		~~- <meta http-equiv="imagetoolbar" content="no" /><![endif]-->~~
66		~~-HTML~~
67		~~- );~~
68		-
69		~~- echo $wgOut->headElement( $this->skin );~~
	72	+ $this->html( 'headelement' );
70	73	?><body<?php if($this->data['body_ondblclick']) { ?> ondblclick="<?php $this->text('body_ondblclick') ?>"<?php } ?>
71	74	<?php if($this->data['body_onload' ]) { ?> onload="<?php $this->text('body_onload') ?>"<?php } ?>
72	75	class="mediawiki <?php $this->text('dir') ?> <?php $this->text('pageclass') ?> <?php $this->text('skinnameclass') ?>">
Index: trunk/phase3/includes/OutputPage.php
—	—	@@ -1724,7 +1724,7 @@
1725	1725	*
1726	1726	* @param $sk Skin The given Skin
1727	1727	*/
1728		~~- public function headElement( Skin $sk , $includeStyle = true ) {~~
	1728	+ public function headElement( Skin $sk, $includeStyle = true ) {
1729	1729	global $wgDocType, $wgDTD, $wgContLanguageCode, $wgOutputEncoding, $wgMimeType;
1730	1730	global $wgXhtmlDefaultNamespace, $wgXhtmlNamespaces;
1731	1731	global $wgContLang, $wgUseTrackbacks, $wgStyleVersion, $wgEnableScriptLoader, $wgHtml5;
—	—	@@ -1764,7 +1764,7 @@
1765	1765	$ret .= implode( "\n", array(
1766	1766	$this->getHeadLinks(),
1767	1767	$this->buildCssLinks(),
1768		~~- $this->getHeadScripts(),~~
	1768	+ $this->getHeadScripts( $sk ),
1769	1769	$this->getHeadItems(),
1770	1770	));
1771	1771	if( $sk->usercss ){
—	—	@@ -1786,19 +1786,24 @@
1787	1787	*
1788	1788	* also adds userjs to the end if enabled:
1789	1789	*/
1790		~~- function getHeadScripts() {~~
1791		~~- global $wgUser, $wgJsMimeType;~~
1792		~~- $sk = $wgUser->getSkin();~~
	1790	+ function getHeadScripts( Skin $sk ) {
	1791	+ global $wgUser, $wgRequest, $wgJsMimeType;
1793	1792
1794	1793	$vars = Skin::makeGlobalVariablesScript( $sk->getSkinName() );
1795	1794
1796	1795	//add user js if enabled:
1797	1796	if( $this->isUserJsAllowed() && $wgUser->isLoggedIn() ) {
1798		~~- $userpage = $wgUser->getUserPage();~~
1799		~~- $userjs = Skin::makeUrl(~~
1800		~~- $userpage->getPrefixedText() . '/' . $sk->getSkinName() . '.js',~~
1801		~~- 'action=raw&ctype=' . $wgJsMimeType );~~
1802		~~- $this->addScriptFile( $userjs );~~
	1797	+ $action = $wgRequest->getVal( 'action', 'view' );
	1798	+ if( $this->mTitle->isJsSubpage() and $sk->userCanPreview( $action ) ) {
	1799	+ # XXX: additional security check/prompt?
	1800	+ $this->addInlineScript( $wgRequest->getText( 'wpTextbox1' ) );
	1801	+ } else {
	1802	+ $userpage = $wgUser->getUserPage();
	1803	+ $userjs = Skin::makeUrl(
	1804	+ $userpage->getPrefixedText() . '/' . $sk->getSkinName() . '.js',
	1805	+ 'action=raw&ctype=' . $wgJsMimeType );
	1806	+ $this->addScriptFile( $userjs );
	1807	+ }
1803	1808	}
1804	1809
1805	1810	return $vars . "\n" . $this->mScripts;
Index: trunk/phase3/includes/SkinTemplate.php
—	—	@@ -87,6 +87,12 @@
88	88	*/
89	89	var $template = 'QuickTemplate';
90	90
	91	+ /**
	92	+ * Whether this skin use OutputPage::headElement() to generate the <head>
	93	+ * tag
	94	+ */
	95	+ var $useHeadElement = false;
	96	+
91	97	/*#@-/
92	98
93	99	/**
—	—	@@ -171,12 +177,48 @@
172	178	$this->userpageUrlDetails = self::makeKnownUrlDetails( $this->userpage );
173	179	}
174	180
175		~~- $this->userjs = $this->userjsprev = false;~~
176		~~- $this->setupUserCss( $out );~~
177		~~- $this->setupUserJs( $out->isUserJsAllowed() );~~
178	181	$this->titletxt = $this->mTitle->getPrefixedText();
179	182	wfProfileOut( __METHOD__ . '-stuff' );
180	183
	184	+ wfProfileIn( __METHOD__ . '-stuff-head' );
	185	+ if ( $this->useHeadElement ) {
	186	+ $pagecss = $this->setupPageCss();
	187	+ if( $pagecss )
	188	+ $out->addInlineStyle( $pagecss );
	189	+ } else {
	190	+ $this->setupUserCss( $out );
	191	+
	192	+ $tpl->set( 'pagecss', $this->setupPageCss() );
	193	+ $tpl->setRef( 'usercss', $this->usercss );
	194	+
	195	+ $this->userjs = $this->userjsprev = false;
	196	+ $this->setupUserJs( $out->isUserJsAllowed() );
	197	+ $tpl->setRef( 'userjs', $this->userjs );
	198	+ $tpl->setRef( 'userjsprev', $this->userjsprev );
	199	+
	200	+ if( $wgUseSiteJs ) {
	201	+ $jsCache = $this->loggedin ? '&smaxage=0' : '';
	202	+ $tpl->set( 'jsvarurl',
	203	+ self::makeUrl( '-',
	204	+ "action=raw$jsCache&gen=js&useskin=" .
	205	+ urlencode( $this->getSkinName() ) ) );
	206	+ } else {
	207	+ $tpl->set( 'jsvarurl', false );
	208	+ }
	209	+
	210	+ $tpl->setRef( 'xhtmldefaultnamespace', $wgXhtmlDefaultNamespace );
	211	+ $tpl->set( 'xhtmlnamespaces', $wgXhtmlNamespaces );
	212	+ $tpl->set( 'headlinks', $out->getHeadLinks() );
	213	+ $tpl->set( 'csslinks', $out->buildCssLinks() );
	214	+
	215	+ if( $wgUseTrackbacks && $out->isArticleRelated() ) {
	216	+ $tpl->set( 'trackbackhtml', $out->getTitle()->trackbackRDF() );
	217	+ } else {
	218	+ $tpl->set( 'trackbackhtml', null );
	219	+ }
	220	+ }
	221	+ wfProfileOut( __METHOD__ . '-stuff-head' );
	222	+
181	223	wfProfileIn( __METHOD__ . '-stuff2' );
182	224	$tpl->set( 'title', $out->getPageTitle() );
183	225	$tpl->set( 'pagetitle', $out->getHTMLTitle() );
—	—	@@ -224,19 +266,10 @@
225	267	} else {
226	268	$tpl->set( 'feeds', false );
227	269	}
228		~~- if( $wgUseTrackbacks && $out->isArticleRelated() ) {~~
229		~~- $tpl->set( 'trackbackhtml', $out->getTitle()->trackbackRDF() );~~
230		~~- } else {~~
231		~~- $tpl->set( 'trackbackhtml', null );~~
232		~~- }~~
233	270
234		~~- $tpl->setRef( 'xhtmldefaultnamespace', $wgXhtmlDefaultNamespace );~~
235		~~- $tpl->set( 'xhtmlnamespaces', $wgXhtmlNamespaces );~~
236	271	$tpl->setRef( 'mimetype', $wgMimeType );
237	272	$tpl->setRef( 'jsmimetype', $wgJsMimeType );
238	273	$tpl->setRef( 'charset', $wgOutputEncoding );
239		~~- $tpl->set( 'headlinks', $out->getHeadLinks() );~~
240		~~- $tpl->set( 'csslinks', $out->buildCssLinks() );~~
241	274	$tpl->setRef( 'wgScript', $wgScript );
242	275	$tpl->setRef( 'skinname', $this->skinname );
243	276	$tpl->set( 'skinclass', get_class( $this ) );
—	—	@@ -271,19 +304,6 @@
272	305	$tpl->setRef( 'userpageurl', $this->userpageUrlDetails['href'] );
273	306	$tpl->set( 'userlang', $wgLang->getCode() );
274	307	$tpl->set( 'userlangattributes', 'lang="' . $wgLang->getCode() . '" xml:lang="' . $wgLang->getCode() . '"' );
275		~~- $tpl->set( 'pagecss', $this->setupPageCss() );~~
276		~~- $tpl->setRef( 'usercss', $this->usercss );~~
277		~~- $tpl->setRef( 'userjs', $this->userjs );~~
278		~~- $tpl->setRef( 'userjsprev', $this->userjsprev );~~
279		~~- if( $wgUseSiteJs ) {~~
280		~~- $jsCache = $this->loggedin ? '&smaxage=0' : '';~~
281		~~- $tpl->set( 'jsvarurl',~~
282		~~- self::makeUrl( '-',~~
283		~~- "action=raw$jsCache&gen=js&useskin=" .~~
284		~~- urlencode( $this->getSkinName() ) ) );~~
285		~~- } else {~~
286		~~- $tpl->set( 'jsvarurl', false );~~
287		~~- }~~
288	308
289	309	$newtalks = $wgUser->getNewMessageLinks();
290	310
—	—	@@ -463,7 +483,11 @@
464	484	$tpl->set( 'nav_urls', $this->buildNavUrls() );
465	485
466	486	// Set the head scripts near the end, in case the above actions resulted in added scripts
467		~~- $tpl->set( 'headscripts', $out->getScript() );~~
	487	+ if ( $this->useHeadElement ) {
	488	+ $tpl->set( 'headelement', $out->headElement( $this ) );
	489	+ } else {
	490	+ $tpl->set( 'headscripts', $out->getScript() );
	491	+ }
468	492
469	493	// original version by hansm
470	494	if( !wfRunHooks( 'SkinTemplateOutputPageBeforeExec', array( &$this, &$tpl ) ) ) {
Index: trunk/phase3/includes/Skin.php
—	—	@@ -463,9 +463,8 @@
464	464	*
465	465	* @param string $action
466	466	* @return bool
467		~~- * @private~~
468	467	*/
469		~~- function userCanPreview( $action ) {~~
	468	+ public function userCanPreview( $action ) {
470	469	global $wgRequest, $wgUser;
471	470
472	471	if( $action != 'submit' )
—	—	@@ -637,9 +636,8 @@
638	637	$action = $wgRequest->getVal( 'action' );
639	638	# If we're previewing the CSS page, use it
640	639	if( $this->mTitle->isCssSubpage() && $this->userCanPreview( $action ) ) {
641		~~- $previewCss = $wgRequest->getText( 'wpTextbox1' );~~
642	640	// @FIXME: properly escape the cdata!
643		~~- $this->usercss = "/<![CDATA[/\n" . $previewCss . "/]]>/";~~
	641	+ $out->addInlineStyle( $wgRequest->getText( 'wpTextbox1' ) );
644	642	} else {
645	643	$out->addStyle( self::makeUrl( $this->userpage . '/' . $this->getSkinName() .'.css',
646	644	'action=raw&ctype=text/css' ) );

Follow-up revisions

Revision	Commit summary	Author	Date
r61728	MonoBook: remove three unused globals which were added in r56770 to support l...	ashley	20:26, 30 January 2010

Status & tagging log

23:53, 29 September 2009 Brion VIBBER (talk | contribs) changed the status of r56770 [removed: new added: ok]