r73811 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r73810‎ \| r73811 \| r73812 >
Date:	15:08, 27 September 2010
Author:	thomasv
Status:	ok (Comments)
Tags:
Comment:	use Resource loader ; adapt scripts
Modified paths:	/trunk/extensions/ProofreadPage/ProofreadPage.php (modified) (history) /trunk/extensions/ProofreadPage/ProofreadPage_body.php (modified) (history) /trunk/extensions/ProofreadPage/proofread.js (modified) (history) /trunk/extensions/ProofreadPage/proofread_article.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ProofreadPage/ProofreadPage_body.php
—	—	@@ -51,11 +51,49 @@
52	52	$wgHooks['LoadExtensionSchemaUpdates'][] = array( &$this, 'schema_update' );
53	53	$wgHooks['EditPage::importFormData'][] = array( &$this, 'importFormData' );
54	54	$wgHooks['OutputPageParserOutput'][] = array( &$this, 'OutputPageParserOutput' );
	55	+ $wgHooks['ResourceLoaderRegisterModules'][] = array( &$this, 'resourceLoaderRegisterModules' );
55	56	wfLoadExtensionMessages( 'ProofreadPage' );
56	57	$this->page_namespace = preg_quote( wfMsgForContent( 'proofreadpage_namespace' ), '/' );
57	58	$this->index_namespace = preg_quote( wfMsgForContent( 'proofreadpage_index_namespace' ), '/' );
58	59	}
59	60
	61	+
	62	+ public static function resourceLoaderRegisterModules() {
	63	+
	64	+ ResourceLoader::register( 'proofreadpage.page',
	65	+ new ResourceLoaderFileModule(
	66	+ array( 'scripts' => 'extensions/ProofreadPage/proofread.js',
	67	+ 'messages' => array( 'proofreadpage_index',
	68	+ 'proofreadpage_nextpage',
	69	+ 'proofreadpage_prevpage',
	70	+ 'proofreadpage_image',
	71	+ 'proofreadpage_header',
	72	+ 'proofreadpage_body',
	73	+ 'proofreadpage_footer',
	74	+ 'proofreadpage_toggleheaders',
	75	+ 'proofreadpage_page_status',
	76	+ 'proofreadpage_quality0_category',
	77	+ 'proofreadpage_quality1_category',
	78	+ 'proofreadpage_quality2_category',
	79	+ 'proofreadpage_quality3_category',
	80	+ 'proofreadpage_quality4_category',
	81	+ )
	82	+ ) ) );
	83	+
	84	+ ResourceLoader::register( 'proofreadpage.article',
	85	+ new ResourceLoaderFileModule(
	86	+ array( 'scripts' => 'extensions/ProofreadPage/proofread_article.js',
	87	+ 'messages'=> array( 'proofreadpage_source', 'proofreadpage_source_message' )
	88	+ ) ) );
	89	+
	90	+ ResourceLoader::register( 'proofreadpage.index',
	91	+ new ResourceLoaderFileModule( array( 'scripts' => 'extensions/ProofreadPage/proofread_index.js' ) ) );
	92	+
	93	+ return true;
	94	+ }
	95	+
	96	+
	97	+
60	98	function schema_update() {
61	99	global $wgExtNewTables;
62	100	$base = dirname(__FILE__);
—	—	@@ -306,42 +344,20 @@
307	345	return true;
308	346	}
309	347
310		-
311	348	function prepareArticle( $out ) {
312		~~- global $wgJsMimeType, $wgScriptPath, $wgRequest, $wgProofreadPageVersion;~~
313		~~- $jsFile = htmlspecialchars( "$wgScriptPath/extensions/ProofreadPage/proofread_article.js?$wgProofreadPageVersion" );~~
314		~~- $out->addScript( <<<EOT~~
315		~~-<script type="$wgJsMimeType" src="$jsFile"></script>~~
316		~~-EOT~~
317		~~- );~~
318		~~- $out->addScript( "<script type=\"{$wgJsMimeType}\">~~
319		~~-var prp_source = \"" . Xml::escapeJsString( wfMsg( 'proofreadpage_source' ) ) . "\";~~
320		~~-var prp_source_message = \"" . Xml::escapeJsString( wfMsg( 'proofreadpage_source_message' ) ) . "\";~~
321		~~-</script>\n"~~
322		~~- );~~
	349	+ $out->addModules( 'proofreadpage.article' );
323	350	$this->displayProofreadingStatus( $out );
324	351	}
325	352
326	353	function prepareIndex( $out ) {
327		~~- global $wgJsMimeType, $wgScriptPath, $wgRequest, $wgProofreadPageVersion;~~
328		~~- $jsFile = htmlspecialchars( "$wgScriptPath/extensions/ProofreadPage/proofread_index.js?$wgProofreadPageVersion" );~~
329		-
330		~~- $out->addScript( <<<EOT~~
331		~~-<script type="$wgJsMimeType" src="$jsFile"></script>~~
332		-
333		~~-EOT~~
334		~~- );~~
335		~~- $out->addScript( "<script type=\"{$wgJsMimeType}\">~~
	354	+ $out->addModules( 'proofreadpage.index' );
	355	+ $out->addInlineScript("
336	356	var prp_index_attributes = \"" . Xml::escapeJsString( wfMsgForContent( 'proofreadpage_index_attributes' ) ) . "\";
337	357	var prp_default_header = \"" . Xml::escapeJsString( wfMsgGetKey( 'proofreadpage_default_header', true, true, false ) ) . "\";
338		~~-var prp_default_footer = \"" . Xml::escapeJsString( wfMsgGetKey( 'proofreadpage_default_footer', true, true, false ) ) . "\";~~
339		~~-</script>\n"~~
340		~~- );~~
	358	+var prp_default_footer = \"" . Xml::escapeJsString( wfMsgGetKey( 'proofreadpage_default_footer', true, true, false ) ) . "\";" );
341	359	}
342	360
343		-
344	361	function preparePage( $out, $m, $isEdit ) {
345		~~- global $wgJsMimeType, $wgScriptPath, $wgRequest, $wgProofreadPageVersion, $wgProofreadPageMaxWidth;~~
346	362	global $wgTitle, $wgUser;
347	363
348	364	if ( !isset( $wgTitle->pr_index_title ) ) {
—	—	@@ -372,8 +388,6 @@
373	389
374	390	list( $index_url, $prev_url, $next_url, $header, $footer, $css, $edit_width ) = $this->navigation( $wgTitle );
375	391
376		~~- $jsFile = htmlspecialchars( "$wgScriptPath/extensions/ProofreadPage/proofread.js?$wgProofreadPageVersion" );~~
377		-
378	392	$jsVars = array(
379	393	'proofreadPageWidth' => intval( $width ),
380	394	'proofreadPageHeight' => intval( $height ),
—	—	@@ -389,32 +403,10 @@
390	404	'proofreadPageUserName' => $wgUser->getName(),
391	405	'proofreadPageCss' => $css,
392	406	);
393		~~- $varScript = Skin::makeVariablesScript( $jsVars );~~
	407	+ $out->addInlineScript( ResourceLoader::makeConfigSetScript( $jsVars ) );
394	408
395		~~- $out->addScript( <<<EOT~~
396		~~-$varScript~~
397		~~-<script type="$wgJsMimeType" src="$jsFile"></script>~~
	409	+ $out->addModules( 'proofreadpage.page' );
398	410
399		~~-EOT~~
400		~~- );~~
401		-
402		~~- // Add messages from i18n~~
403		~~- $out->addScript( "<script type=\"{$wgJsMimeType}\">~~
404		~~-var proofreadPageMessageIndex = \"" . Xml::escapeJsString( wfMsg( 'proofreadpage_index' ) ) . "\";~~
405		~~-var proofreadPageMessageNextPage = \"" . Xml::escapeJsString( wfMsg( 'proofreadpage_nextpage' ) ) . "\";~~
406		~~-var proofreadPageMessagePrevPage = \"" . Xml::escapeJsString( wfMsg( 'proofreadpage_prevpage' ) ) . "\";~~
407		~~-var proofreadPageMessageImage = \"" . Xml::escapeJsString( wfMsg( 'proofreadpage_image' ) ) . "\";~~
408		~~-var proofreadPageMessageHeader = \"" . Xml::escapeJsString( wfMsg( 'proofreadpage_header' ) ) . "\";~~
409		~~-var proofreadPageMessagePageBody = \"" . Xml::escapeJsString( wfMsg( 'proofreadpage_body' ) ) . "\";~~
410		~~-var proofreadPageMessageFooter = \"" . Xml::escapeJsString( wfMsg( 'proofreadpage_footer' ) ) . "\";~~
411		~~-var proofreadPageMessageToggleHeaders = \"" . Xml::escapeJsString( wfMsg( 'proofreadpage_toggleheaders' ) ) . "\";~~
412		~~-var proofreadPageMessageStatus = \"" . Xml::escapeJsString( wfMsg( 'proofreadpage_page_status' ) ) . "\";~~
413		~~-var proofreadPageMessageQuality0 = \"" . Xml::escapeJsString( wfMsgForContent( 'proofreadpage_quality0_category' ) ) . "\";~~
414		~~-var proofreadPageMessageQuality1 = \"" . Xml::escapeJsString( wfMsgForContent( 'proofreadpage_quality1_category' ) ) . "\";~~
415		~~-var proofreadPageMessageQuality2 = \"" . Xml::escapeJsString( wfMsgForContent( 'proofreadpage_quality2_category' ) ) . "\";~~
416		~~-var proofreadPageMessageQuality3 = \"" . Xml::escapeJsString( wfMsgForContent( 'proofreadpage_quality3_category' ) ) . "\";~~
417		~~-var proofreadPageMessageQuality4 = \"" . Xml::escapeJsString( wfMsgForContent( 'proofreadpage_quality4_category' ) ) . "\";~~
418		~~-</script>\n" );~~
419	411	return true;
420	412	}
421	413
Index: trunk/extensions/ProofreadPage/proofread.js
—	—	@@ -18,7 +18,7 @@
19	19	var view_url = self.proofreadPageThumbURL.replace( '##WIDTH##', '' + self.proofreadPageWidth );
20	20	b[0].innerHTML = b[0].innerHTML + '<li id="ca-image">' +
21	21	'<span><a href="' + escapeQuotesHTML( view_url ) + '">' +
22		~~- escapeQuotesHTML( proofreadPageMessageImage ) + '</a></span></li>';~~
	22	+ escapeQuotesHTML( mediaWiki.msg.get( 'proofreadpage_image' ) ) + '</a></span></li>';
23	23	}
24	24
25	25	if( self.proofreadPageIndexURL ) {
—	—	@@ -26,16 +26,16 @@
27	27	'<span><a href="' + escapeQuotesHTML( proofreadPageIndexURL ) +
28	28	'" title="' + escapeQuotesHTML( proofreadPageMessageIndex ) + '">' +
29	29	'<img src="' + wgScriptPath + '/extensions/ProofreadPage/uparrow.png" alt="' +
30		~~- escapeQuotesHTML( proofreadPageMessageIndex ) +~~
	30	+ escapeQuotesHTML( mediaWiki.msg.get( 'proofreadpage_index' ) ) +
31	31	'" width="15" height="15" /></a></span></li>';
32	32	}
33	33
34	34	if( self.proofreadPageNextURL ) {
35	35	b[0].innerHTML = '<li id="ca-next">' +
36	36	'<span><a href="' + escapeQuotesHTML( self.proofreadPageNextURL ) +
37		~~- '" title="' + escapeQuotesHTML( proofreadPageMessageNextPage ) + '">' +~~
	37	+ '" title="' + escapeQuotesHTML( mediaWiki.msg.get( 'proofreadpage_nextpage' ) ) + '">' +
38	38	'<img src="' + wgScriptPath + '/extensions/ProofreadPage/rightarrow.png" alt="' +
39		~~- escapeQuotesHTML( proofreadPageMessageNextPage ) +~~
	39	+ escapeQuotesHTML( mediaWiki.msg.get( 'proofreadpage_nextpage' ) ) +
40	40	'" width="15" height="15" /></a></span></li>' +
41	41	b[0].innerHTML;
42	42	}
—	—	@@ -43,9 +43,9 @@
44	44	if( self.proofreadPagePrevURL ) {
45	45	b[0].innerHTML = '<li id="ca-prev">' +
46	46	'<span><a href="' + escapeQuotesHTML( self.proofreadPagePrevURL ) +
47		~~- '" title="' + escapeQuotesHTML( proofreadPageMessagePrevPage ) + '">' +~~
	47	+ '" title="' + escapeQuotesHTML( mediaWiki.msg.get( 'proofreadpage_prevpage' ) ) + '">' +
48	48	'<img src="' + wgScriptPath + '/extensions/ProofreadPage/leftarrow.png" alt="' +
49		~~- escapeQuotesHTML( proofreadPageMessagePrevPage ) +~~
	49	+ escapeQuotesHTML( mediaWiki.msg.get( 'proofreadpage_prevpage' ) ) +
50	50	'" width="15" height="15" /></a></span></li>' +
51	51	b[0].innerHTML;
52	52	}
—	—	@@ -95,7 +95,7 @@
96	96	pageBody = text;
97	97	pageFooter = proofreadPageFooter;
98	98	if( document.editform ) {
99		~~- document.editform.elements['wpSummary'].value = '/* ' + proofreadPageMessageQuality1 + ' */ ';~~
	99	+ document.editform.elements['wpSummary'].value = "/* " + mediaWiki.msg.get( 'proofreadpage_quality1_category' ) + " */ ";
100	100	}
101	101	}
102	102	}
—	—	@@ -166,18 +166,20 @@
167	167	pageHeader = pageHeader.split( '&' ).join( '&' );
168	168	pageFooter = pageFooter.split( '&' ).join( '&' );
169	169
	170	+ if ( ! self.proofreadpage_show_headers ) headers_style = 'display:none'; else headers_style='';
	171	+
170	172	container.innerHTML = '' +
171		~~- '<div id="prp_header" style="">' +~~
	173	+ '<div id="prp_header" style="' + headers_style + '">' +
172	174	'<span style="color:gray;font-size:80%;line-height:100%;">' +
173		~~- escapeQuotesHTML( proofreadPageMessageHeader ) + '</span>' +~~
	175	+ escapeQuotesHTML( mediaWiki.msg.get( 'proofreadpage_header' ) ) + '</span>' +
174	176	'<textarea name="wpHeaderTextbox" rows="2" cols="80" tabindex=1>' + pageHeader + '</textarea><br />' +
175	177	'<span style="color:gray;font-size:80%;line-height:100%;">' +
176		~~- escapeQuotesHTML( proofreadPageMessagePageBody ) + '</span></div>' +~~
	178	+ escapeQuotesHTML( mediaWiki.msg.get( 'proofreadpage_body' ) ) + '</span></div>' +
177	179	'<textarea name="wpTextbox1" id="wpTextbox1" tabindex=1 style="height:' + ( self.DisplayHeight - 6 ) + 'px;">' +
178	180	pageBody + '</textarea>' +
179		~~- '<div id="prp_footer" style="">' +~~
	181	+ '<div id="prp_footer" style="' + headers_style + '">' +
180	182	'<span style="color:gray;font-size:80%;line-height:100%;">' +
181		~~- escapeQuotesHTML( proofreadPageMessageFooter ) + '</span><br />' +~~
	183	+ escapeQuotesHTML( mediaWiki.msg.get( 'proofreadpage_footer' ) ) + '</span><br />' +
182	184	'<textarea name="wpFooterTextbox" rows="2" cols="80" tabindex=1>' +
183	185	pageFooter + '</textarea></div>';
184	186	}
—	—	@@ -599,7 +601,7 @@
600	602	}
601	603	}
602	604
603		~~-function pr_zoom( delta ) {~~
	605	+self.pr_zoom = function(delta) {
604	606	if ( delta == 0 ) {
605	607	// reduce width by 20 pixels in order to prevent horizontal scrollbar
606	608	// from showing up
—	—	@@ -774,7 +776,7 @@
775	777	pr_container.addEventListener( 'DOMMouseScroll', pr_zoom_wheel, false );
776	778	}
777	779	pr_container.onmousewheel = pr_zoom_wheel; // IE, Opera.
778		~~- hookEvent( 'load', function() { pr_load_image( view_url ); } );~~
	780	+ pr_load_image(view_url);
779	781	}
780	782
781	783	table.setAttribute( 'id', 'textBoxTable' );
—	—	@@ -798,9 +800,6 @@
799	801	pr_make_edit_area( self.text_container, new_text.value );
800	802	var copywarn = document.getElementById( 'editpage-copywarn' );
801	803	f.insertBefore( table, copywarn );
802		~~- if ( !self.proofreadpage_show_headers ) {~~
803		~~- hookEvent( 'load', pr_toggle_visibility );~~
804		~~- }~~
805	804	} else {
806	805	self.text_container.appendChild( new_text );
807	806	f.appendChild( self.table );
—	—	@@ -816,8 +815,8 @@
817	816	image.className = 'mw-toolbar-editbutton';
818	817	image.src = wgScriptPath + '/extensions/ProofreadPage/button_category_plus.png';
819	818	image.border = 0;
820		~~- image.alt = proofreadPageMessageToggleHeaders;~~
821		~~- image.title = proofreadPageMessageToggleHeaders;~~
	819	+ image.alt = mediaWiki.msg.get( 'proofreadpage_toggleheaders' );
	820	+ image.title = mediaWiki.msg.get( 'proofreadpage_toggleheaders' );
822	821	image.style.cursor = 'pointer';
823	822	image.onclick = pr_toggle_visibility;
824	823
—	—	@@ -954,25 +953,25 @@
955	954	hookEvent( 'load', pr_initzoom );
956	955
957	956	/* Quality buttons */
958		~~-function pr_add_quality( form, value ) {~~
	957	+self.pr_add_quality = function( form, value ) {
959	958	self.proofreadpage_quality = value;
960	959	self.proofreadpage_username = proofreadPageUserName;
961	960	var text = '';
962	961	switch( value ) {
963	962	case 0:
964		~~- text = proofreadPageMessageQuality0;~~
	963	+ text = mediaWiki.msg.get( 'proofreadpage_quality0_category' );
965	964	break;
966	965	case 1:
967		~~- text = proofreadPageMessageQuality1;~~
	966	+ text = mediaWiki.msg.get( 'proofreadpage_quality1_category' );
968	967	break;
969	968	case 2:
970		~~- text = proofreadPageMessageQuality2;~~
	969	+ text = mediaWiki.msg.get( 'proofreadpage_quality2_category' );
971	970	break;
972	971	case 3:
973		~~- text = proofreadPageMessageQuality3;~~
	972	+ text = mediaWiki.msg.get( 'proofreadpage_quality3_category' );
974	973	break;
975	974	case 4:
976		~~- text = proofreadPageMessageQuality4;~~
	975	+ text = mediaWiki.msg.get( 'proofreadpage_quality4_category' );
977	976	break;
978	977	}
979	978	form.elements['wpSummary'].value = '/* ' + text + ' */ ';
—	—	@@ -1004,7 +1003,7 @@
1005	1004	+'<span class="quality1"> <input type="radio" name="quality" value=1 onclick="pr_add_quality(this.form,1)" tabindex=4> </span>'
1006	1005	+'<span class="quality3"> <input type="radio" name="quality" value=3 onclick="pr_add_quality(this.form,3)" tabindex=4> </span>'
1007	1006	+'<span class="quality4"> <input type="radio" name="quality" value=4 onclick="pr_add_quality(this.form,4)" tabindex=4> </span>';
1008		~~- f.innerHTML = f.innerHTML + ' ' + escapeQuotesHTML( proofreadPageMessageStatus );~~
	1007	+ f.innerHTML = f.innerHTML + ' ' + escapeQuotesHTML( mediaWiki.msg.get( 'proofreadpage_page_status' ) );
1009	1008
1010	1009	if( !( ( self.proofreadpage_quality == 4 ) \|\| ( ( self.proofreadpage_quality == 3 ) && ( self.proofreadpage_username != proofreadPageUserName ) ) ) ) {
1011	1010	document.editform.quality[4].parentNode.style.cssText = 'display:none';
Index: trunk/extensions/ProofreadPage/proofread_article.js
—	—	@@ -13,8 +13,8 @@
14	14	return;
15	15	}
16	16	q.removeChild( href );
17		~~- href.innerHTML = prp_source;~~
18		~~- href.setAttribute( 'title', prp_source_message );~~
	17	+ href.innerHTML = mediaWiki.msg.get( 'proofreadpage_source' );
	18	+ href.setAttribute( 'title', mediaWiki.msg.get( 'proofreadpage_source_message' ) );
19	19	var new_li = document.createElement( 'li' );
20	20	new_li.appendChild( href );
21	21	var new_span = document.createElement( 'span' );
Index: trunk/extensions/ProofreadPage/ProofreadPage.php
—	—	@@ -53,9 +53,6 @@
54	54	$wgSpecialPages['PagesWithoutScans'] = 'PagesWithoutScans';
55	55	$wgSpecialPageGroups['PagesWithoutScans'] = 'maintenance';
56	56
57		~~-# Bump the version number every time you change proofread.js~~
58		~~-$wgProofreadPageVersion = 27;~~
59		-
60	57	# Group allowed to modify pagequality
61	58	$wgGroupPermissions['user']['pagequality'] = true;
62	59

Follow-up revisions

Revision	Commit summary	Author	Date
r73812	ProofreadPage: follow-up to r73811 - tweak coding style	ashley	15:14, 27 September 2010
r73877	follow up to r73811	thomasv	09:37, 28 September 2010

Comments

#Comment by Catrope (talk | contribs) 15:23, 27 September 2010

+		ResourceLoader::register( 'proofreadpage.page',

The convention (which we should document, oops) is to prefix extension modules with ext., so that would make this ext.proofreadpage.page.

You can also register multiple modules with one call, like this:

ResourceLoader::register( array( 'module1' => array( 'scripts' => 'foo.js', ... ), 'module2' => array( 'scripts' => 'bar.js', ... ) );

+		$out->addInlineScript("
 var prp_index_attributes = \"" . Xml::escapeJsString( wfMsgForContent( 'proofreadpage_index_attributes' ) ) . "\";
 var prp_default_header = \"" . Xml::escapeJsString( wfMsgGetKey( 'proofreadpage_default_header', true, true, false ) ) . "\";
-var prp_default_footer = \"" . Xml::escapeJsString( wfMsgGetKey( 'proofreadpage_default_footer', true, true, false ) ) . "\";
-</script>\n"
-				 );
+var prp_default_footer = \"" . Xml::escapeJsString( wfMsgGetKey( 'proofreadpage_default_footer', true, true, false ) ) . "\";" );

What do you still need this for?

+		$out->addInlineScript( ResourceLoader::makeConfigSetScript( $jsVars ) );

Hmm. I think the resource loader should have nicer support for this.

#Comment by ThomasV (talk | contribs) 20:43, 27 September 2010

These 3 variables are not callled with wfMsg(), but with wfMsgForContent() and wfMsgGetKey() ; these are configuration messages, and they should not be translated. I do not know if resource loader has support for that.

#Comment by Catrope (talk | contribs) 11:49, 28 September 2010

Not yet, no, but it should. Filed as bug 25349

#Comment by ThomasV (talk | contribs) 09:51, 28 September 2010

I tried to prefix module names with "ext." as you suggest, but the result is that they are no longer loaded. did I miss something ?

#Comment by Catrope (talk | contribs) 11:39, 28 September 2010

Did you change the name everywhere it's being used? Did you action=purge the page you were looking at?

#Comment by ThomasV (talk | contribs) 14:16, 28 September 2010

all right, I fixed it with $document.ready(), see r73889

Status & tagging log

15:23, 27 September 2010 Catrope (talk | contribs) changed the status of r73811 [removed: new added: ok]