r104397 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r104396‎ \| r104397 \| r104398 >
Date:	05:52, 28 November 2011
Author:	johnduhart
Status:	deferred (Comments)
Tags:
Comment:	Cleanup the AddThis extension to conform to MediaWiki standards
Modified paths:	/trunk/extensions/AddThis/AddThis.body.php (modified) (history) /trunk/extensions/AddThis/AddThis.php (modified) (history)

Diff [purge]

Index: trunk/extensions/AddThis/AddThis.body.php
—	—	@@ -8,101 +8,119 @@
9	9	*/
10	10	class AddThis {
11	11
12		-/**
13		~~- * Parser hook for the <addthis /> tag extension.~~
14		- *
15		~~- */~~
	12	+ /**
	13	+ * Parser hook for the <addthis /> tag extension.
	14	+ *
	15	+ * @param $parser
	16	+ * @return string
	17	+ */
16	18	public static function parserHook( $parser ) {
17		~~- global $wgAddThispubid, $wgAddThisHeader, $wgAddThisMain, $wgAddThisMainpage, $wgRequest, $wgAddThisHServ, $wgAddThisBackground, $wgAddThisBorder;~~
18		~~- # Localisation for "Share"~~
	19	+ global $wgAddThispubid, $wgAddThisHServ, $wgAddThisBackground, $wgAddThisBorder;
	20	+
	21	+ # Localisation for "Share"
19	22	$share = wfMsg( 'addthis' );
20		~~- # Output AddThis widget~~
21		~~- $output .='<!-- AddThis Button BEGIN -->~~
	23	+ # Output AddThis widget
	24	+ $output ='<!-- AddThis Button BEGIN -->
22	25	<div class="addthis_toolbox addthis_default_style" id="addthistoolbar" style="background:'.$wgAddThisBackground.'; border-color:'.$wgAddThisBorder.';">
23	26	<a href="http://www.addthis.com/bookmark.php?v=250&pubid=ra-4eb75def4ec6488b" class="addthis_button_compact"> ' . $share . '</a><span class="addthis_separator"> </span>';
	27	+
24	28	foreach ( $wgAddThisHServ as $n => $a ) {
25		~~- if (true === isset($wgAddThisHServ[$n]["attribs"])) {~~
	29	+ if ( isset( $wgAddThisHServ[$n]["attribs"] ) ) {
26	30	$output .= '<a class="addthis_button_'.$wgAddThisHServ[$n]["service"].'" '.$wgAddThisHServ[$n]["attribs"].'></a>';
27		~~- }~~
28		~~- else {~~
29		~~- $output .= '<a class="addthis_button_'.$wgAddThisHServ[$n]["service"].'"></a>';~~
30		~~- }~~
	31	+ } else {
	32	+ $output .= '<a class="addthis_button_'.$wgAddThisHServ[$n]["service"].'"></a>';
	33	+ }
31	34	}
32	35	$output .='</div>
33	36	<script type="text/javascript" src="http://s7.addthis.com/js/250/addthis_widget.js#pubid='.$wgAddThispubid.'"></script>
34	37	<!-- AddThis Button END -->';
	38	+
35	39	return $output;
36		~~- }~~
	40	+ }
37	41
	42	+ /**
	43	+ * Function for sidebar portlet
	44	+ *
	45	+ * @param $skin
	46	+ * @param $bar
	47	+ * @return array\|bool\|string
	48	+ */
	49	+ public static function AddThisSidebar( $skin, &$bar ) {
	50	+ global $wgOut, $wgAddThispubid, $wgAddThisSidebar, $wgAddThisSBServ;
38	51
39		-/**
40		~~- * Function for sidebar portlet~~
41		- *
42		~~- */~~
43		~~- static function AddThisSidebar( $skin, &$bar ) {~~
44		~~- global $wgOut, $wgAddThispubid, $wgAddThisSidebar, $wgAddThisSBServ;~~
45		~~- # Load css stylesheet~~
46		~~- $wgOut->addModuleStyles( 'ext.addThis' );~~
47		~~- # Check setting to enable/disable sidebar portlet~~
48		~~- if(strtolower($wgAddThisSidebar) == 'true') {~~
49		~~- # Output AddThis widget~~
	52	+ # Load css stylesheet
	53	+ $wgOut->addModuleStyles( 'ext.addThis' );
	54	+ # Check setting to enable/disable sidebar portlet
	55	+ if ( $wgAddThisSidebar ) {
	56	+ # Output AddThis widget
50	57	$bar['addthis'] = '<!-- AddThis Button BEGIN -->
51	58	<div class="addthis_toolbox addthis_default_style" id="addthissidebar">';
	59	+
52	60	foreach ( $wgAddThisSBServ as $n => $a ) {
53		~~- if (true === isset($wgAddThisSBServ[$n]["attribs"])) {~~
	61	+ if ( isset( $wgAddThisSBServ[$n]["attribs"] ) ) {
54	62	$bar['addthis'] .= '<a class="addthis_button_'.$wgAddThisSBServ[$n]["service"].'" '.$wgAddThisSBServ[$n]["attribs"].'></a>';
55		~~- }~~
56		~~- else {~~
57		~~- $bar['addthis'] .= '<a class="addthis_button_'.$wgAddThisSBServ[$n]["service"].'"></a>';~~
58		~~- }~~
	63	+ } else {
	64	+ $bar['addthis'] .= '<a class="addthis_button_'.$wgAddThisSBServ[$n]["service"].'"></a>';
59	65	}
	66	+ }
	67	+
60	68	$bar['addthis'] .= '</div>
61	69	<script type="text/javascript" src="http://s7.addthis.com/js/250/addthis_widget.js#pubid='.$wgAddThispubid.'"></script>
62	70	<!-- AddThis Button END -->';
	71	+
	72	+ // FIXME: This is a hook, should we really be returning a string here?
63	73	return $bar;
64	74	}
65	75	return true;
66	76	}
67	77
	78	+ /**
	79	+ * Function for article header toolbar
	80	+ *
	81	+ * @param $article
	82	+ * @param $outputDone
	83	+ * @param $pcache
	84	+ * @return bool
	85	+ */
	86	+ public static function AddThisHeader( &$article, &$outputDone, &$pcache ) {
	87	+ global $wgOut, $wgAddThispubid, $wgAddThisHeader, $wgAddThisMain,
	88	+ $wgRequest, $wgAddThisHServ, $wgAddThisBackground, $wgAddThisBorder;
68	89
69		-/**
70		~~- * Function for article header toolbar~~
71		- *
72		~~- */~~
73		~~- public static function AddThisHeader( &$article, &$outputDone, &$pcache ) {~~
74		~~- global $wgOut, $wgAddThispubid, $wgAddThisHeader, $wgAddThisMain, $wgAddThisMainpage, $wgRequest, $wgAddThisHServ, $wgAddThisBackground, $wgAddThisBorder;~~
75		~~- # Localisation for "Share"~~
76		~~- $share = wfMsg( 'addthis' );~~
77		~~- # Check if page is in main namespace~~
78		~~- $title = $article->getTitle();~~
79		~~- # Check setting to enable/disable article header tooblar~~
80		~~- if(strtolower($wgAddThisHeader) == 'true') {~~
81		~~- # Check if article is mainpage set by [[MediaWiki:Mainpage]]~~
82		~~- if ($wgRequest->getText( 'title' )==str_replace( ' ', '_', wfMsg( 'mainpage' ) ) ) {~~
83		~~- # Check setting to enable/disable article header toolbar on mainpage~~
84		~~- if(strtolower($wgAddThisMain) == 'false') {~~
	90	+ # Localisation for "Share"
	91	+ $share = wfMsg( 'addthis' );
	92	+ # Check if page is in main namespace
	93	+ $title = $article->getTitle();
	94	+ # Check setting to enable/disable article header tooblar
	95	+ if ( $wgAddThisHeader ) {
	96	+ # Check if article is mainpage set by [[MediaWiki:Mainpage]]
	97	+ if ( $wgRequest->getText( 'title' )==str_replace( ' ', '_', wfMsg( 'mainpage' ) ) ) {
	98	+ # Check setting to enable/disable article header toolbar on mainpage
	99	+ if( !$wgAddThisMain ) {
85	100	return true;
86		~~- }~~
87	101	}
88		~~- # Check if page is in content namespace~~
	102	+ }
	103	+
	104	+ # Check if page is in content namespace
89	105	if ( MWNamespace::isContent( $title->getNamespace() ) ) {
90		~~- # Output AddThis widget~~
	106	+ # Output AddThis widget
91	107	$wgOut->addHTML('<!-- AddThis Button BEGIN -->
92	108	<div class="addthis_toolbox addthis_default_style" id="addthistoolbar" style="background:'.$wgAddThisBackground.'; border-color:'.$wgAddThisBorder.';">
93	109	<a href="http://www.addthis.com/bookmark.php?v=250&pubid=ra-4eb75def4ec6488b" class="addthis_button_compact"> ' . $share . '</a><span class="addthis_separator"> </span>');
	110	+
94	111	foreach ( $wgAddThisHServ as $n => $a ) {
95	112	if (true === isset($wgAddThisHServ[$n]["attribs"])) {
96	113	$wgOut->addHTML('<a class="addthis_button_'.$wgAddThisHServ[$n]["service"].'" '.$wgAddThisHServ[$n]["attribs"].'></a>');
97		~~- }~~
98		~~- else {~~
99		~~- $wgOut->addHTML('<a class="addthis_button_'.$wgAddThisHServ[$n]["service"].'"></a>');~~
100		~~- }~~
	114	+ } else {
	115	+ $wgOut->addHTML('<a class="addthis_button_'.$wgAddThisHServ[$n]["service"].'"></a>');
	116	+ }
101	117	}
	118	+
102	119	$wgOut->addHTML('</div>
103	120	<script type="text/javascript" src="http://s7.addthis.com/js/250/addthis_widget.js#pubid='.$wgAddThispubid.'"></script>
104	121	<!-- AddThis Button END -->');
105	122	}
106	123	}
107		~~- return true;~~
	124	+
	125	+ return true;
108	126	}
109	127	}
\ No newline at end of file
Index: trunk/extensions/AddThis/AddThis.php
—	—	@@ -117,9 +117,9 @@
118	118	# Default values for most options
119	119	$wgAddThisBackground = '#f6f6f6';
120	120	$wgAddThisBorder = '#a7d7f9';
121		~~-$wgAddThisSidebar = 'true';~~
122		~~-$wgAddThisHeader = 'true';~~
123		~~-$wgAddThisMain = 'true';~~
	121	+$wgAddThisSidebar = true;
	122	+$wgAddThisHeader = true;
	123	+$wgAddThisMain = true;
124	124
125	125	# Sidebar settings
126	126	$wgAddThisSBServ = array(
—	—	@@ -184,7 +184,6 @@
185	185	'url' => 'http://www.mediawiki.org/wiki/Extension:AddThis',
186	186	);
187	187
188		-
189	188	/**
190	189	* Register class and localisations
191	190	*
—	—	@@ -193,29 +192,27 @@
194	193	$wgAutoloadClasses['AddThis'] = $dir . 'AddThis.body.php';
195	194	$wgExtensionMessagesFiles['AddThis'] = $dir . 'AddThis.i18n.php';
196	195
197		-
198	196	$wgResourceModules['ext.addThis'] = array(
199	197	'styles' => 'addThis.css',
200	198	'localBasePath' => dirname( __FILE__ ),
201	199	'remoteExtPath' => 'AddThis',
202	200	);
203	201
204		-
205	202	/**
206	203	* Hooks
207	204	*
208	205	*/
209	206	$wgHooks['ArticleViewHeader'][] = 'AddThis::AddThisHeader';
210		~~-$wgHooks['ParserFirstCallInit'][] = 'addThisHeaderTag';~~
	207	+$wgHooks['ParserFirstCallInit'][] = 'efAddThisHeaderTag';
211	208	$wgHooks['SkinBuildSidebar'][] = 'AddThis::AddThisSidebar';
212	209
213		-
214	210	/**
215	211	* Register parser hook
216	212	*
217	213	* @param $parser Parser
	214	+ * @return bool
218	215	*/
219		~~-function addThisHeaderTag( &$parser ) {~~
	216	+function efAddThisHeaderTag( &$parser ) {
220	217	$parser->setHook( 'addthis', 'AddThis::parserHook' );
221	218	return true;
222	219	}
\ No newline at end of file

Comments

#Comment by Nikerabbit (talk | contribs) 09:10, 29 November 2011

There must be a better way to check if Title is main page.

Status & tagging log

15:24, 28 November 2011 Johnduhart (talk | contribs) changed the status of r104397 [removed: new added: deferred]