MediaWiki r78156 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:r78155‎ | r78156 (on ViewVC)‎ | r78157 >
Date:23:29, 9 December 2010
Author:krinkle
Status:ok (Comments)
Tags:
Comment:
Ported jsMsg to mw.util; Fixing bugs and modernising mediawiki.action.watch.ajax.js a little more

* Using the raw element instead of jQuery object to get href. This way it's the complete url instead of what could potentially be a relative path (window.location.href is best passed a complete url)
* Adding 'mediawiki.legacy.ajax' as dependency for mediawiki.action.watch.ajax as it is and has been for a while.
* Ported jsMsg (legacy.wikibits) to mw.util.jsMessage()
Modified paths:

Diff [purge]

Index: trunk/phase3/resources/mediawiki.action/mediawiki.action.watch.ajax.js
@@ -1,7 +1,6 @@
22 /**
33 * Animate watch/unwatch links to use asynchronous API requests to
44 * watch pages, rather than clicking on links. Requires jQuery.
5 - * Uses jsMsg() from wikibits.js.
65 */
76
87 if ( typeof wgAjaxWatch === 'undefined' || !wgAjaxWatch ) {
@@ -37,16 +36,16 @@
3837 wgAjaxWatch.$links.trigger( 'mw-ajaxwatch', [response.title, 'unwatch', $link] );
3938 } else {
4039 // Either we got an error code or it just plain broke.
41 - window.location.href = $link.attr( 'href' );
 40+ window.location.href = $link[0].href;
4241 return;
4342 }
4443
45 - jsMsg( response.message, 'watch' );
 44+ mw.util.jsMessage( response.message, 'watch' );
4645
4746 // Bug 12395 - update the watch checkbox on edit pages when the
4847 // page is watched or unwatched via the tab.
4948 if( response.watched !== undefined ) {
50 - $( '#wpWatchthis' ).attr( 'checked', '1' );
 49+ $( '#wpWatchthis' ).attr( 'checked', 'checked' );
5150 } else {
5251 $( '#wpWatchthis' ).removeAttr( 'checked' );
5352 }
@@ -74,7 +73,7 @@
7574 $links.click( function( event ) {
7675 var $link = $( this );
7776
78 - if( wgAjaxWatch.supported === false || !wgEnableWriteAPI || !wfSupportsAjax() ) {
 77+ if( wgAjaxWatch.supported === false || !mw.config.get( 'wgEnableWriteAPI' ) || !wfSupportsAjax() ) {
7978 // Lazy initialization so we don't toss up
8079 // ActiveX warnings on initial page load
8180 // for IE 6 users with security settings.
@@ -83,8 +82,8 @@
8483 }
8584
8685 wgAjaxWatch.setLinkText( $link, $link.data( 'action' ) + 'ing' );
87 - $.getJSON( wgScriptPath
88 - + '/api' + wgScriptExtension + '?action=watch&format=json&title='
 86+ $.getJSON( mw.config.get( 'wgScriptPath' )
 87+ + '/api' + mw.config.get( 'wgScriptExtension' ) + '?action=watch&format=json&title='
8988 + encodeURIComponent( $link.data( 'target' ) )
9089 + ( $link.data( 'action' ) == 'unwatch' ? '&unwatch' : '' ),
9190 function( data, textStatus, xhr ) {
@@ -107,8 +106,8 @@
108107 $link.data( 'action', otheraction );
109108 wgAjaxWatch.setLinkText( $link, otheraction );
110109 $link.attr( 'href', $link.attr( 'href' ).replace( '&action=' + action , '&action=' + otheraction ) );
111 - if( $link.parents( 'li' ).attr( 'id' ) == 'ca-' + action ) {
112 - $link.parents( 'li' ).attr( 'id', 'ca-' + otheraction );
 110+ if( $link.closest( 'li' ).attr( 'id' ) == 'ca-' + action ) {
 111+ $link.closest( 'li' ).attr( 'id', 'ca-' + otheraction );
113112 // update the link text with the new message
114113 $link.text( mediaWiki.msg( otheraction ) );
115114 }
Index: trunk/phase3/resources/mediawiki.util/mediawiki.util.js
@@ -314,6 +314,43 @@
315315
316316 return $item.get( 0 );
317317 }
 318+ },
 319+
 320+ /**
 321+ * Add a little box at the top of the screen to inform the user of
 322+ * something, replacing any previous message.
 323+ *
 324+ * @param message mixed DOM-element or HTML to be put inside the message box
 325+ * @param className string Used in adding a class; should be different for each
 326+ * call to allow CSS/JS to hide different boxes. null = no class used.
 327+ * @return Boolean True on success, false on failure
 328+ */
 329+ 'jsMessage' : function( message, className ) {
 330+ // We special-case skin structures provided by the software. Skins that
 331+ // choose to abandon or significantly modify our formatting can just define
 332+ // an mw-js-message div to start with.
 333+ var $messageDiv = $( '#mw-js-message' );
 334+ if ( !$messageDiv.length ) {
 335+ $messageDiv = $( '<div id="mw-js-message">' );
 336+ if ( mw.util.$content.parent().length ) {
 337+ mw.util.$content.parent().prepend( $messageDiv );
 338+ } else {
 339+ return false;
 340+ }
 341+ }
 342+
 343+ $messageDiv.show();
 344+ if ( className ) {
 345+ $messageDiv.attr( 'class', 'mw-js-message-' + className );
 346+ }
 347+
 348+ if ( typeof message === 'object' ) {
 349+ $messageDiv.empty();
 350+ $messageDiv.append( message ); // Append new content
 351+ } else {
 352+ $messageDiv.html( message );
 353+ }
 354+ return true;
318355 }
319356
320357 };
Index: trunk/phase3/resources/Resources.php
@@ -343,7 +343,7 @@
344344 ),
345345 'mediawiki.action.watch.ajax' => array(
346346 'scripts' => 'resources/mediawiki.action/mediawiki.action.watch.ajax.js',
347 - 'dependencies' => 'mediawiki.util',
 347+ 'dependencies' => array( 'mediawiki.util', 'mediawiki.legacy.ajax' ),
348348 ),
349349 'mediawiki.special.preferences' => array(
350350 'scripts' => 'resources/mediawiki.special/mediawiki.special.preferences.js',

Comments

#Comment by Helder.wiki (talk | contribs)   21:51, 5 July 2011

This isn't deployed to Wikimedia yet, is it?

Will this be part of MW 1.18?

#Comment by Krinkle (talk | contribs)   22:05, 5 July 2011

Not on Wikimedia / 1.17. Will be in 1.18. See also Branch points

Status & tagging log