r86872 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r86871‎ \| r86872 \| r86873 >
Date:	17:37, 25 April 2011
Author:	happy-melon
Status:	reverted (Comments)
Tags:
Comment:	Implement an interface and abstract class to hold the widely-reused get(Request\|User\|Title\|Lang\|Skin\|Output) accessors for objects acting as a context source. Article is rather messier because both getTitle() and getUser() are in use for other things, and Article::$mTitle is in extremely wide use both within Article.php and outside.
Modified paths:	/trunk/phase3/includes/Action.php (modified) (history) /trunk/phase3/includes/AutoLoader.php (modified) (history) /trunk/phase3/includes/HTMLForm.php (modified) (history) /trunk/phase3/includes/OutputPage.php (modified) (history) /trunk/phase3/includes/RequestContext.php (modified) (history) /trunk/phase3/includes/Skin.php (modified) (history) /trunk/phase3/includes/SpecialPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/HTMLForm.php
—	—	@@ -53,7 +53,7 @@
54	54	*
55	55	* TODO: Document 'section' / 'subsection' stuff
56	56	*/
57		~~-class HTMLForm {~~
	57	+class HTMLForm extends ContextSource {
58	58
59	59	# A mapping of 'type' inputs onto standard HTMLFormField subclasses
60	60	static $typeMappings = array(
—	—	@@ -102,7 +102,6 @@
103	103	protected $mSubmitText;
104	104	protected $mSubmitTooltip;
105	105
106		~~- protected $mContext; // <! RequestContext~~
107	106	protected $mTitle;
108	107	protected $mMethod = 'post';
109	108
—	—	@@ -121,10 +120,11 @@
122	121	*/
123	122	public function __construct( $descriptor, /RequestContext/ $context = null, $messagePrefix = '' ) {
124	123	if( $context instanceof RequestContext ){
125		~~- $this->mContext = $context;~~
	124	+ $this->setContext( $context );
126	125	$this->mTitle = false; // We don't need them to set a title
127	126	$this->mMessagePrefix = $messagePrefix;
128	127	} else {
	128	+ $this->setContext( RequestContext::getMain() );
129	129	// B/C since 1.18
130	130	if( is_string( $context ) && $messagePrefix === '' ){
131	131	// it's actually $messagePrefix
—	—	@@ -623,41 +623,11 @@
624	624	*/
625	625	function getTitle() {
626	626	return $this->mTitle === false
627		~~- ? $this->getContext()->title~~
	627	+ ? parent::getTitle()
628	628	: $this->mTitle;
629	629	}
630	630
631	631	/**
632		~~- * @return RequestContext~~
633		~~- */~~
634		~~- public function getContext(){~~
635		~~- return $this->mContext instanceof RequestContext~~
636		~~- ? $this->mContext~~
637		~~- : RequestContext::getMain();~~
638		~~- }~~
639		-
640		- /**
641		~~- * @return OutputPage~~
642		~~- */~~
643		~~- public function getOutput(){~~
644		~~- return $this->getContext()->output;~~
645		~~- }~~
646		-
647		- /**
648		~~- * @return WebRequest~~
649		~~- */~~
650		~~- public function getRequest(){~~
651		~~- return $this->getContext()->request;~~
652		~~- }~~
653		-
654		- /**
655		~~- * @return User~~
656		~~- */~~
657		~~- public function getUser(){~~
658		~~- return $this->getContext()->user;~~
659		~~- }~~
660		-
661		- /**
662	632	* Set the method used to submit the form
663	633	* @param $method String
664	634	*/
Index: trunk/phase3/includes/RequestContext.php
—	—	@@ -7,7 +7,7 @@
8	8	* @file
9	9	*/
10	10
11		~~-class RequestContext {~~
	11	+class RequestContext implements IContextSource {
12	12	private $mRequest; // / WebRequest object
13	13	private $mTitle; // / Title object
14	14	private $mOutput; // / OutputPage object
—	—	@@ -205,3 +205,134 @@
206	206	}
207	207	}
208	208
	209	+/**
	210	+ * Interface for objects which can provide a context on request.
	211	+ */
	212	+interface IContextSource {
	213	+
	214	+ /**
	215	+ * Get the WebRequest object
	216	+ *
	217	+ * @return WebRequest
	218	+ */
	219	+ public function getRequest();
	220	+
	221	+ /**
	222	+ * Get the Title object
	223	+ *
	224	+ * @return Title
	225	+ */
	226	+ public function getTitle();
	227	+
	228	+ /**
	229	+ * Get the OutputPage object
	230	+ *
	231	+ * @return OutputPage object
	232	+ */
	233	+ public function getOutput();
	234	+
	235	+ /**
	236	+ * Get the User object
	237	+ *
	238	+ * @return User
	239	+ */
	240	+ public function getUser();
	241	+
	242	+ /**
	243	+ * Get the Language object
	244	+ *
	245	+ * @return Language
	246	+ */
	247	+ public function getLang();
	248	+
	249	+ /**
	250	+ * Get the Skin object
	251	+ *
	252	+ * @return Skin
	253	+ */
	254	+ public function getSkin();
	255	+}
	256	+
	257	+/**
	258	+ * The simplest way of implementing IContextSource is to hold a RequestContext as a
	259	+ * member variable and provide accessors to it.
	260	+ */
	261	+abstract class ContextSource implements IContextSource {
	262	+
	263	+ /**
	264	+ * @var RequestContext
	265	+ */
	266	+ private $context;
	267	+
	268	+ /**
	269	+ * Get the RequestContext object
	270	+ *
	271	+ * @return RequestContext
	272	+ */
	273	+ public function getContext() {
	274	+ return $this->context;
	275	+ }
	276	+
	277	+ /**
	278	+ * Set the RequestContext object
	279	+ *
	280	+ * @param $context RequestContext
	281	+ */
	282	+ public function setContext( RequestContext $context ) {
	283	+ $this->context = $context;
	284	+ }
	285	+
	286	+ /**
	287	+ * Get the WebRequest object
	288	+ *
	289	+ * @return WebRequest
	290	+ */
	291	+ public function getRequest() {
	292	+ return $this->context->getRequest();
	293	+ }
	294	+
	295	+ /**
	296	+ * Get the Title object
	297	+ *
	298	+ * @return Title
	299	+ */
	300	+ public function getTitle() {
	301	+ return $this->context->getTitle();
	302	+ }
	303	+
	304	+ /**
	305	+ * Get the OutputPage object
	306	+ *
	307	+ * @return OutputPage object
	308	+ */
	309	+ public function getOutput() {
	310	+ return $this->context->getOutput();
	311	+ }
	312	+
	313	+ /**
	314	+ * Get the User object
	315	+ *
	316	+ * @return User
	317	+ */
	318	+ public function getUser() {
	319	+ return $this->context->getUser();
	320	+ }
	321	+
	322	+ /**
	323	+ * Get the Language object
	324	+ *
	325	+ * @return Language
	326	+ */
	327	+ public function getLang() {
	328	+ return $this->context->getLang();
	329	+ }
	330	+
	331	+ /**
	332	+ * Get the Skin object
	333	+ *
	334	+ * @return Skin
	335	+ */
	336	+ public function getSkin() {
	337	+ return $this->context->getSkin();
	338	+ }
	339	+}
\ No newline at end of file
Index: trunk/phase3/includes/OutputPage.php
—	—	@@ -18,7 +18,7 @@
19	19	*
20	20	* @todo document
21	21	*/
22		~~-class OutputPage {~~
	22	+class OutputPage extends ContextSource {
23	23	/// Should be private. Used with addMeta() which adds <meta>
24	24	var $mMetatags = array();
25	25
—	—	@@ -220,11 +220,13 @@
221	221	* a OutputPage tied to that context.
222	222	*/
223	223	function __construct( RequestContext $context = null ) {
224		~~- if ( !isset($context) ) {~~
	224	+ if ( $context === null ) {
225	225	# Extensions should use `new RequestContext` instead of `new OutputPage` now.
226	226	wfDeprecated( __METHOD__ );
	227	+ $this->setContext( RequestContext::getMain() );
	228	+ } else {
	229	+ $this->setContext( $context );
227	230	}
228		~~- $this->mContext = $context;~~
229	231	}
230	232
231	233	/**
—	—	@@ -764,29 +766,6 @@
765	767	}
766	768
767	769	/**
768		~~- * Get the RequestContext used in this instance~~
769		- *
770		~~- * @return RequestContext~~
771		~~- */~~
772		~~- private function getContext() {~~
773		~~- if ( !isset($this->mContext) ) {~~
774		~~- wfDebug( __METHOD__ . " called and \$mContext is null. Using RequestContext::getMain(); for sanity\n" );~~
775		~~- $this->mContext = RequestContext::getMain();~~
776		~~- }~~
777		~~- return $this->mContext;~~
778		~~- }~~
779		-
780		- /**
781		~~- * Get the WebRequest being used for this instance~~
782		- *
783		~~- * @return WebRequest~~
784		~~- * @since 1.18~~
785		~~- */~~
786		~~- public function getRequest() {~~
787		~~- return $this->getContext()->getRequest();~~
788		~~- }~~
789		-
790		- /**
791	770	* Set the Title object to use
792	771	*
793	772	* @param $t Title object
—	—	@@ -796,35 +775,6 @@
797	776	}
798	777
799	778	/**
800		~~- * Get the Title object used in this instance~~
801		- *
802		~~- * @return Title~~
803		~~- */~~
804		~~- public function getTitle() {~~
805		~~- return $this->getContext()->getTitle();~~
806		~~- }~~
807		-
808		- /**
809		~~- * Get the User object used in this instance~~
810		- *
811		~~- * @return User~~
812		~~- * @since 1.18~~
813		~~- */~~
814		~~- public function getUser() {~~
815		~~- return $this->getContext()->getUser();~~
816		~~- }~~
817		-
818		- /**
819		~~- * Get the Skin object used to render this instance~~
820		- *
821		~~- * @return Skin~~
822		~~- * @since 1.18~~
823		~~- */~~
824		~~- public function getSkin() {~~
825		~~- return $this->getContext()->getSkin();~~
826		~~- }~~
827		-
828		- /**
829	779	* Replace the subtile with $str
830	780	*
831	781	* @param $str String: new value of the subtitle
—	—	@@ -2177,7 +2127,7 @@
2178	2128	? 'lag-warn-normal'
2179	2129	: 'lag-warn-high';
2180	2130	$wrap = Html::rawElement( 'div', array( 'class' => "mw-{$message}" ), "\n$1\n" );
2181		~~- $this->wrapWikiMsg( "$wrap\n", array( $message, $this->getContext()->getLang()->formatNum( $lag ) ) );~~
	2131	+ $this->wrapWikiMsg( "$wrap\n", array( $message, $this->getLang()->formatNum( $lag ) ) );
2182	2132	}
2183	2133	}
2184	2134
—	—	@@ -2323,7 +2273,7 @@
2324	2274	$dir = wfUILang()->getDir();
2325	2275	$bodyAttrs['class'] = "mediawiki $dir";
2326	2276
2327		~~- if ( $this->getContext()->getLang()->capitalizeAllNouns() ) {~~
	2277	+ if ( $this->getLang()->capitalizeAllNouns() ) {
2328	2278	# A <body> class is probably not the best way to do this . . .
2329	2279	$bodyAttrs['class'] .= ' capitalize-all-nouns';
2330	2280	}
Index: trunk/phase3/includes/AutoLoader.php
—	—	@@ -49,6 +49,7 @@
50	50	'ConfEditorParseError' => 'includes/ConfEditor.php',
51	51	'ConfEditorToken' => 'includes/ConfEditor.php',
52	52	'ConstantDependency' => 'includes/CacheDependency.php',
	53	+ 'ContextSource' => 'includes/RequestContext.php',
53	54	'Cookie' => 'includes/Cookie.php',
54	55	'CookieJar' => 'includes/Cookie.php',
55	56	'CreativeCommonsRdf' => 'includes/Metadata.php',
—	—	@@ -123,6 +124,7 @@
124	125	'HTMLTextField' => 'includes/HTMLForm.php',
125	126	'Http' => 'includes/HttpFunctions.php',
126	127	'HttpRequest' => 'includes/HttpFunctions.old.php',
	128	+ 'IContextSource' => 'includes/RequestContext.php',
127	129	'IcuCollation' => 'includes/Collation.php',
128	130	'ImageGallery' => 'includes/ImageGallery.php',
129	131	'ImageHistoryList' => 'includes/ImagePage.php',
Index: trunk/phase3/includes/Skin.php
—	—	@@ -15,7 +15,7 @@
16	16	*
17	17	* @ingroup Skins
18	18	*/
19		~~-abstract class Skin {~~
	19	+abstract class Skin extends ContextSource {
20	20	/**#@+
21	21	* @private
22	22	*/
—	—	@@ -190,10 +190,8 @@
191	191	* Preload the existence of three commonly-requested pages in a single query
192	192	*/
193	193	function preloadExistence() {
194		~~- $user = $this->getContext()->getUser();~~
195		-
196	194	// User/talk link
197		~~- $titles = array( $user->getUserPage(), $user->getTalkPage() );~~
	195	+ $titles = array( $this->getUser()->getUserPage(), $this->getUser()->getTalkPage() );
198	196
199	197	// Other tab link
200	198	if ( $this->getTitle()->getNamespace() == NS_SPECIAL ) {
—	—	@@ -213,7 +211,7 @@
214	212	* Set some local variables
215	213	*/
216	214	protected function setMembers() {
217		~~- $this->userpage = $this->getContext()->getUser()->getUserPage()->getPrefixedText();~~
	215	+ $this->userpage = $this->getUser()->getUserPage()->getPrefixedText();
218	216	$this->usercss = false;
219	217	}
220	218
—	—	@@ -227,43 +225,18 @@
228	226	}
229	227
230	228	/**
231		~~- * Set the RequestContext used in this instance~~
232		- *
233		~~- * @param RequestContext $context~~
234		~~- */~~
235		~~- public function setContext( RequestContext $context ) {~~
236		~~- $this->mContext = $context;~~
237		~~- }~~
238		-
239		- /**
240	229	* Get the RequestContext used in this instance
241	230	*
242	231	* @return RequestContext
243	232	*/
244	233	public function getContext() {
245		~~- if ( !isset($this->mContext) ) {~~
	234	+ if ( !parent::getContext() instanceof RequestContext ) {
246	235	wfDebug( __METHOD__ . " called and \$mContext is null. Using RequestContext::getMain(); for sanity\n" );
247		~~- $this->mContext = RequestContext::getMain();~~
	236	+ $this->setContext( RequestContext::getMain() );
248	237	}
249		~~- return $this->mContext;~~
	238	+ return parent::getContext();
250	239	}
251	240
252		~~- /** Get the title~~
253		- *
254		~~- * @return Title~~
255		~~- */~~
256		~~- public function getTitle() {~~
257		~~- return $this->getContext()->getTitle();~~
258		~~- }~~
259		-
260		~~- /** Get the user~~
261		- *
262		~~- * @return User~~
263		~~- */~~
264		~~- public function getUser() {~~
265		~~- return $this->getContext()->getUser();~~
266		~~- }~~
267		-
268	241	/**
269	242	* Set the "relevant" title
270	243	* @see self::getRelevantTitle()
—	—	@@ -355,7 +328,7 @@
356	329	if ( $action != 'submit' ) {
357	330	return false;
358	331	}
359		~~- if ( !$this->getContext()->getRequest()->wasPosted() ) {~~
	332	+ if ( !$this->getRequest()->wasPosted() ) {
360	333	return false;
361	334	}
362	335	if ( !$this->getTitle()->userCanEditCssSubpage() ) {
—	—	@@ -365,8 +338,8 @@
366	339	return false;
367	340	}
368	341
369		~~- return $this->getContext()->getUser()->matchEditToken(~~
370		~~- $this->getContext()->getRequest()->getVal( 'wpEditToken' ) );~~
	342	+ return $this->getUser()->matchEditToken(
	343	+ $this->getRequest()->getVal( 'wpEditToken' ) );
371	344	}
372	345
373	346	/**
—	—	@@ -428,16 +401,16 @@
429	402	// Per-site custom styles
430	403	if ( $wgUseSiteCss ) {
431	404	$out->addModuleStyles( array( 'site', 'noscript' ) );
432		~~- if( $this->getContext()->getUser()->isLoggedIn() ){~~
	405	+ if( $this->getUser()->isLoggedIn() ){
433	406	$out->addModuleStyles( 'user.groups' );
434	407	}
435	408	}
436	409
437	410	// Per-user custom styles
438	411	if ( $wgAllowUserCss ) {
439		~~- if ( $this->getTitle()->isCssSubpage() && $this->userCanPreview( $this->getContext()->getRequest()->getVal( 'action' ) ) ) {~~
	412	+ if ( $this->getTitle()->isCssSubpage() && $this->userCanPreview( $this->getRequest()->getVal( 'action' ) ) ) {
440	413	// @FIXME: properly escape the cdata!
441		~~- $out->addInlineStyle( $this->getContext()->getRequest()->getText( 'wpTextbox1' ) );~~
	414	+ $out->addInlineStyle( $this->getRequest()->getText( 'wpTextbox1' ) );
442	415	} else {
443	416	$out->addModuleStyles( 'user' );
444	417	}
—	—	@@ -527,7 +500,7 @@
528	501	global $wgUseCategoryBrowser, $wgContLang;
529	502
530	503	if( $out === null ){
531		~~- $out = $this->getContext()->output;~~
	504	+ $out = $this->getOutput();
532	505	}
533	506
534	507	if ( count( $out->mCategoryLinks ) == 0 ) {
—	—	@@ -558,7 +531,7 @@
559	532
560	533	# Hidden categories
561	534	if ( isset( $allCats['hidden'] ) ) {
562		~~- if ( $this->getContext()->getUser()->getBoolOption( 'showhiddencats' ) ) {~~
	535	+ if ( $this->getUser()->getBoolOption( 'showhiddencats' ) ) {
563	536	$class = 'mw-hidden-cats-user-shown';
564	537	} elseif ( $this->getTitle()->getNamespace() == NS_CATEGORY ) {
565	538	$class = 'mw-hidden-cats-ns-shown';
—	—	@@ -629,7 +602,7 @@
630	603
631	604	// Check what we're showing
632	605	$allCats = $out->getCategoryLinks();
633		~~- $showHidden = $this->getContext()->getUser()->getBoolOption( 'showhiddencats' ) \|\|~~
	606	+ $showHidden = $this->getUser()->getBoolOption( 'showhiddencats' ) \|\|
634	607	$this->getTitle()->getNamespace() == NS_CATEGORY;
635	608
636	609	if ( empty( $allCats['normal'] ) && !( !empty( $allCats['hidden'] ) && $showHidden ) ) {
—	—	@@ -763,14 +736,14 @@
764	737	}
765	738
766	739	function getUndeleteLink() {
767		~~- $action = $this->getContext()->getRequest()->getVal( 'action', 'view' );~~
	740	+ $action = $this->getRequest()->getVal( 'action', 'view' );
768	741
769		~~- if ( $this->getContext()->getUser()->isAllowed( 'deletedhistory' ) &&~~
	742	+ if ( $this->getUser()->isAllowed( 'deletedhistory' ) &&
770	743	( $this->getTitle()->getArticleId() == 0 \|\| $action == 'history' ) ) {
771	744	$n = $this->getTitle()->isDeleted();
772	745
773	746	if ( $n ) {
774		~~- if ( $this->getContext()->getUser()->isAllowed( 'undelete' ) ) {~~
	747	+ if ( $this->getUser()->isAllowed( 'undelete' ) ) {
775	748	$msg = 'thisisdeleted';
776	749	} else {
777	750	$msg = 'viewdeleted';
—	—	@@ -780,7 +753,7 @@
781	754	$msg,
782	755	Linker::link(
783	756	SpecialPage::getTitleFor( 'Undelete', $this->getTitle()->getPrefixedDBkey() ),
784		~~- wfMsgExt( 'restorelink', array( 'parsemag', 'escape' ), $this->getContext()->getLang()->formatNum( $n ) ),~~
	757	+ wfMsgExt( 'restorelink', array( 'parsemag', 'escape' ), $this->getLang()->formatNum( $n ) ),
785	758	array(),
786	759	array(),
787	760	array( 'known', 'noclasses' )
—	—	@@ -793,10 +766,10 @@
794	767	}
795	768
796	769	/**
797		~~- * The format without an explicit $out argument is deprecated~~
	770	+ * The format with an explicit $out argument is deprecated
798	771	*/
799	772	function subPageSubtitle( OutputPage $out=null ) {
800		~~- $out = $this->getContext()->getOutput();~~
	773	+ $out = $this->getOutput();
801	774	$subpages = '';
802	775
803	776	if ( !wfRunHooks( 'SkinSubPageSubtitle', array( &$subpages, $this, $out ) ) ) {
—	—	@@ -818,7 +791,7 @@
819	792	$linkObj = Title::newFromText( $growinglink );
820	793
821	794	if ( is_object( $linkObj ) && $linkObj->exists() ) {
822		~~- $getlink = $this->link(~~
	795	+ $getlink = Linker::link(
823	796	$linkObj,
824	797	htmlspecialchars( $display ),
825	798	array(),
—	—	@@ -868,7 +841,7 @@
869	842	global $wgRightsPage, $wgRightsUrl, $wgRightsText;
870	843
871	844	if ( $type == 'detect' ) {
872		~~- $diff = $this->getContext()->getRequest()->getVal( 'diff' );~~
	845	+ $diff = $this->getRequest()->getVal( 'diff' );
873	846
874	847	if ( is_null( $diff ) && !$this->isRevisionCurrent() && wfMsgForContent( 'history_copyright' ) !== '-' ) {
875	848	$type = 'history';
—	—	@@ -964,8 +937,8 @@
965	938	}
966	939
967	940	if ( $timestamp ) {
968		~~- $d = $this->getContext()->getLang()->date( $timestamp, true );~~
969		~~- $t = $this->getContext()->getLang()->time( $timestamp, true );~~
	941	+ $d = $this->getLang()->date( $timestamp, true );
	942	+ $t = $this->getLang()->time( $timestamp, true );
970	943	$s = ' ' . wfMsg( 'lastmodifiedat', $d, $t );
971	944	} else {
972	945	$s = '';
—	—	@@ -1092,7 +1065,7 @@
1093	1066
1094	1067	function showEmailUser( $id ) {
1095	1068	$targetUser = User::newFromId( $id );
1096		~~- return $this->getContext()->getUser()->canSendEmail() && # the sending user must have a confirmed email address~~
	1069	+ return $this->getUser()->canSendEmail() && # the sending user must have a confirmed email address
1097	1070	$targetUser->canReceiveEmail(); # the target user must have a confirmed email address and allow emails from users
1098	1071	}
1099	1072
—	—	@@ -1214,7 +1187,7 @@
1215	1188	global $parserMemc, $wgEnableSidebarCache, $wgSidebarCacheExpiry;
1216	1189	wfProfileIn( __METHOD__ );
1217	1190
1218		~~- $key = wfMemcKey( 'sidebar', $this->getContext()->getLang()->getCode() );~~
	1191	+ $key = wfMemcKey( 'sidebar', $this->getLang()->getCode() );
1219	1192
1220	1193	if ( $wgEnableSidebarCache ) {
1221	1194	$cachedsidebar = $parserMemc->get( $key );
—	—	@@ -1350,9 +1323,9 @@
1351	1324	* @return MediaWiki message or if no new talk page messages, nothing
1352	1325	*/
1353	1326	function getNewtalks() {
1354		~~- $out = $this->getContext()->getOutput();~~
	1327	+ $out = $this->getOutput();
1355	1328
1356		~~- $newtalks = $this->getContext()->getUser()->getNewMessageLinks();~~
	1329	+ $newtalks = $this->getUser()->getNewMessageLinks();
1357	1330	$ntl = '';
1358	1331
1359	1332	if ( count( $newtalks ) == 1 && $newtalks[0]['wiki'] === wfWikiID() ) {
—	—	@@ -1360,7 +1333,7 @@
1361	1334	$userTalkTitle = $userTitle->getTalkPage();
1362	1335
1363	1336	if ( !$userTalkTitle->equals( $out->getTitle() ) ) {
1364		~~- $newMessagesLink = $this->link(~~
	1337	+ $newMessagesLink = Linker::link(
1365	1338	$userTalkTitle,
1366	1339	wfMsgHtml( 'newmessageslink' ),
1367	1340	array(),
—	—	@@ -1368,7 +1341,7 @@
1369	1342	array( 'known', 'noclasses' )
1370	1343	);
1371	1344
1372		~~- $newMessagesDiffLink = $this->link(~~
	1345	+ $newMessagesDiffLink = Linker::link(
1373	1346	$userTalkTitle,
1374	1347	wfMsgHtml( 'newmessagesdifflink' ),
1375	1348	array(),
—	—	@@ -1447,7 +1420,7 @@
1448	1421	}
1449	1422
1450	1423	if ( $needParse ) {
1451		~~- $parsed = $this->getContext()->getOutput()->parse( $notice );~~
	1424	+ $parsed = $this->getOutput()->parse( $notice );
1452	1425	$parserMemc->set( $key, array( 'html' => $parsed, 'hash' => md5( $notice ) ), 600 );
1453	1426	$notice = $parsed;
1454	1427	}
—	—	@@ -1487,7 +1460,7 @@
1488	1461	$siteNotice = '';
1489	1462
1490	1463	if ( wfRunHooks( 'SiteNoticeBefore', array( &$siteNotice, $this ) ) ) {
1491		~~- if ( is_object( $this->getContext()->getUser() ) && $this->getContext()->getUser()->isLoggedIn() ) {~~
	1464	+ if ( $this->getUser() instanceof User && $this->getUser()->isLoggedIn() ) {
1492	1465	$siteNotice = $this->getCachedNotice( 'sitenotice' );
1493	1466	} else {
1494	1467	$anonNotice = $this->getCachedNotice( 'anonnotice' );
Index: trunk/phase3/includes/SpecialPage.php
—	—	@@ -27,7 +27,7 @@
28	28	* page list.
29	29	* @ingroup SpecialPage
30	30	*/
31		~~-class SpecialPage {~~
	31	+class SpecialPage extends ContextSource {
32	32
33	33	// The canonical name of this special page
34	34	// Also used for the default <h1> heading, @see getDescription()
—	—	@@ -572,16 +572,6 @@
573	573	function getTitle( $subpage = false ) {
574	574	return self::getTitleFor( $this->mName, $subpage );
575	575	}
576		-
577		- /**
578		~~- * Sets the context this SpecialPage is executed in~~
579		- *
580		~~- * @param $context RequestContext~~
581		~~- * @since 1.18~~
582		~~- */~~
583		~~- public function setContext( $context ) {~~
584		~~- $this->mContext = $context;~~
585		~~- }~~
586	576
587	577	/**
588	578	* Gets the context this SpecialPage is executed in
—	—	@@ -590,8 +580,8 @@
591	581	* @since 1.18
592	582	*/
593	583	public function getContext() {
594		~~- if ( $this->mContext instanceof RequestContext ) {~~
595		~~- return $this->mContext;~~
	584	+ if ( parent::getContext() instanceof RequestContext ) {
	585	+ return parent::getContext();
596	586	} else {
597	587	wfDebug( __METHOD__ . " called and \$mContext is null. Return RequestContext::getMain(); for sanity\n" );
598	588	return RequestContext::getMain();
—	—	@@ -599,46 +589,6 @@
600	590	}
601	591
602	592	/**
603		~~- * Get the WebRequest being used for this instance~~
604		- *
605		~~- * @return WebRequest~~
606		~~- * @since 1.18~~
607		~~- */~~
608		~~- public function getRequest() {~~
609		~~- return $this->getContext()->getRequest();~~
610		~~- }~~
611		-
612		- /**
613		~~- * Get the OutputPage being used for this instance~~
614		- *
615		~~- * @return OutputPage~~
616		~~- * @since 1.18~~
617		~~- */~~
618		~~- public function getOutput() {~~
619		~~- return $this->getContext()->getOutput();~~
620		~~- }~~
621		-
622		- /**
623		~~- * Shortcut to get the skin being used for this instance~~
624		- *
625		~~- * @return User~~
626		~~- * @since 1.18~~
627		~~- */~~
628		~~- public function getUser() {~~
629		~~- return $this->getContext()->getUser();~~
630		~~- }~~
631		-
632		- /**
633		~~- * Shortcut to get the skin being used for this instance~~
634		- *
635		~~- * @return Skin~~
636		~~- * @since 1.18~~
637		~~- */~~
638		~~- public function getSkin() {~~
639		~~- return $this->getContext()->getSkin();~~
640		~~- }~~
641		-
642		- /**
643	593	* Return the full title, including $par
644	594	*
645	595	* @return Title
Index: trunk/phase3/includes/Action.php
—	—	@@ -23,16 +23,12 @@
24	24	*
25	25	* @file
26	26	*/
27		~~-abstract class Action {~~
	27	+abstract class Action extends ContextSource {
28	28
29	29	// Page on which we're performing the action
30	30	// @var Article
31	31	protected $page;
32	32
33		~~- // RequestContext if specified; otherwise we'll use the Context from the Page~~
34		~~- // @var RequestContext~~
35		~~- protected $context;~~
36		-
37	33	// The fields used to create the HTMLForm
38	34	// @var Array
39	35	protected $fields;
—	—	@@ -91,76 +87,13 @@
92	88	}
93	89
94	90	/**
95		~~- * Get the RequestContext in use here~~
96		~~- * @return RequestContext~~
97		~~- */~~
98		~~- protected final function getContext() {~~
99		~~- if ( $this->context instanceof RequestContext ) {~~
100		~~- return $this->context;~~
101		~~- }~~
102		~~- return $this->page->getContext();~~
103		~~- }~~
104		-
105		- /**
106		~~- * Get the WebRequest being used for this instance~~
107		- *
108		~~- * @return WebRequest~~
109		~~- */~~
110		~~- protected final function getRequest() {~~
111		~~- return $this->getContext()->request;~~
112		~~- }~~
113		-
114		- /**
115		~~- * Get the OutputPage being used for this instance~~
116		- *
117		~~- * @return OutputPage~~
118		~~- */~~
119		~~- protected final function getOutput() {~~
120		~~- return $this->getContext()->output;~~
121		~~- }~~
122		-
123		- /**
124		~~- * Shortcut to get the User being used for this instance~~
125		- *
126		~~- * @return User~~
127		~~- */~~
128		~~- protected final function getUser() {~~
129		~~- return $this->getContext()->user;~~
130		~~- }~~
131		-
132		- /**
133		~~- * Shortcut to get the Skin being used for this instance~~
134		- *
135		~~- * @return Skin~~
136		~~- */~~
137		~~- protected final function getSkin() {~~
138		~~- return $this->getContext()->skin;~~
139		~~- }~~
140		-
141		- /**
142		~~- * Shortcut to get the user Language being used for this instance~~
143		- *
144		~~- * @return Skin~~
145		~~- */~~
146		~~- protected final function getLang() {~~
147		~~- return $this->getContext()->lang;~~
148		~~- }~~
149		-
150		- /**
151		~~- * Shortcut to get the Title object from the page~~
152		~~- * @return Title~~
153		~~- */~~
154		~~- protected final function getTitle() {~~
155		~~- return $this->page->getTitle();~~
156		~~- }~~
157		-
158		- /**
159	91	* Protected constructor: use Action::factory( $action, $page ) to actually build
160	92	* these things in the real world
161	93	* @param Article $page
162	94	*/
163	95	protected function __construct( Article $page ) {
164	96	$this->page = $page;
	97	+ $this->setContext( $page->getContext() );
165	98	}
166	99
167	100	/**
—	—	@@ -349,7 +282,7 @@
350	283	public function execute( array $data = null, $captureErrors = true ) {
351	284	try {
352	285	// Set a new context so output doesn't leak.
353		~~- $this->context = clone $this->page->getContext();~~
	286	+ $this->setContext( clone $this->page->getContext() );
354	287
355	288	// This will throw exceptions if there's a problem
356	289	$this->checkCanExecute( $this->getUser() );
—	—	@@ -431,10 +364,11 @@
432	365	public function execute( array $data = null, $captureErrors = true ) {
433	366	try {
434	367	// Set a new context so output doesn't leak.
435		~~- $this->context = clone $this->page->getContext();~~
	368	+ $context = clone $this->page->getContext();
436	369	if ( is_array( $data ) ) {
437		~~- $this->context->setRequest( new FauxRequest( $data, false ) );~~
	370	+ $context->setRequest( new FauxRequest( $data, false ) );
438	371	}
	372	+ $this->setContext( $context );
439	373
440	374	// This will throw exceptions if there's a problem
441	375	$this->checkCanExecute( $this->getUser() );

Follow-up revisions

Revision	Commit summary	Author	Date
r86875	Revert r86872: Breaks LiquidThreads page moves with the below failure. Thread...	siebrand	18:20, 25 April 2011
r89406	Start unpicking r85288 (magic __get() accessor for RequestContext). Instead,...	happy-melon	10:54, 3 June 2011
r90901	Follow-up r89408, r86872: restore IContextSource and ContextSource, to be mor...	happy-melon	19:38, 27 June 2011

Comments

#Comment by Siebrand (talk | contribs) 18:18, 25 April 2011

This breaks LiquidThreads page moves with the below failure. Threads are lost and nowhere to be found any more. Reverting.

[25-Apr-2011 18:12:45] /wiki/Special:MoveThread/Thread:User_talk:Siebrand/test/One_new_message: Exception: MWNamespace::getTalk does not make any sense for given namespace -1
#0 /www/w/includes/Namespace.php(81): MWNamespace::isMethodValidFor(-1, 'MWNamespace::ge...')
#1 /www/w/includes/WatchedItem.php(73): MWNamespace::getTalk(-1)
#2 /www/w/includes/User.php(2304): WatchedItem->addWatch()
#3 /www/w/includes/actions/WatchAction.php(53): User->addWatch(Object(Title))
#4 /www/w/includes/Action.php(376): WatchAction->onView()
#5 /www/w/extensions/LiquidThreads/classes/Thread.php(115): FormlessAction->execute()
#6 /www/w/extensions/LiquidThreads/classes/Thread.php(435): Thread::create(Object(Article), Object(Article), NULL, 1, 'One new message')
#7 /www/w/extensions/LiquidThreads/classes/Thread.php(414): Thread->leaveTrace('move test', Object(Title), Object(Title))
#8 /www/w/extensions/LiquidThreads/pages/SpecialMoveThread.php(107): Thread->moveToPage(Object(Title), 'move test', true)
#9 [internal function]: SpecialMoveThread->trySubmit(Array)
#10 /www/w/includes/HTMLForm.php(279): call_user_func(Array, Array)
#11 /www/w/includes/HTMLForm.php(228): HTMLForm->trySubmit()
#12 /www/w/includes/HTMLForm.php(242): HTMLForm->tryAuthorizedSubmit()
#13 /www/w/extensions/LiquidThreads/pages/ThreadActionPage.php(37): HTMLForm->show()
#14 /www/w/includes/SpecialPageFactory.php(459): ThreadActionPage->execute('Thread:User_tal...')
#15 /www/w/includes/Wiki.php(252): SpecialPageFactory::executePath(Object(Title), Object(RequestContext))
#16 /www/w/includes/Wiki.php(98): MediaWiki->handleSpecialCases()
#17 /www/w/index.php(145): MediaWiki->performRequestForTitle(NULL)
#18 {main}

#Comment by Happy-melon (talk | contribs) 23:00, 25 April 2011

Yikes...

Status & tagging log

18:18, 25 April 2011 Siebrand (talk | contribs) changed the status of r86872 [removed: new added: reverted]