r107354 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r107353‎ | r107354 | r107355 >
Date:01:21, 27 December 2011
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
[mediawiki.action.watch.ajax.js] Rewrite using mw.Api and fixes bug 27146
* Use mw.Api and new it's new .watch() as of r107350
* No longer get title from url, use wgPageName instead
* No longer simple queryParam check for action in url,
now supports wgActionPaths as well.
* Simplification and speed up (less back and forth between
functions and jQuery-ism). Previously it had $(..) with several
.add() calls. Now doing one call.
* Uses mw.util.tooltipAccessKeyRegexp instead of local regex
* Uses jQuery.fn.text instead of jQuery.fn.html for link text message

* Should fix bug 27146 (previously a failed attempt in r82498)
* Previousy worked on in r88527, r88511, r78150, r78147

* minor whitespace/comment fix in mediawiki.util.js/mediawiki.page.startup.js
Modified paths:
  • /trunk/phase3/resources/mediawiki.action/mediawiki.action.watch.ajax.js (modified) (history)
  • /trunk/phase3/resources/mediawiki.page/mediawiki.page.startup.js (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.util.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki.action/mediawiki.action.watch.ajax.js
@@ -1,174 +1,150 @@
22 /**
33 * Animate watch/unwatch links to use asynchronous API requests to
4 - * watch pages, rather than clicking on links. Requires jQuery.
 4+ * watch pages, rather than navigating to a different URI.
55 */
6 -( function( $ ) {
7 -var $links;
 6+( function ( $, mw, undefined ) {
87
9 -var setLinkText = function( $link, action ) {
10 - if ( action == 'watch' || action == 'unwatch' ) {
11 - // save the accesskey from the title
12 - var keyCommand = $link.attr( 'title' ).match( /\[.*?\]$/ ) ? $link.attr( 'title' ).match( /\[.*?\]$/ )[0] : '';
13 - $link.attr( 'title', mw.msg( 'tooltip-ca-' + action ) + ' ' + keyCommand );
14 - }
15 - if ( $link.data( 'icon' ) ) {
16 - $link.attr( 'alt', mw.msg( action ) );
17 - if ( action == 'watching' || action == 'unwatching' ) {
 8+/**
 9+ * Update the link text, link href attribute and (if applicable)
 10+ * "loading" class.
 11+ *
 12+ * @param $link {jQuery} Anchor tag of (un)watch link
 13+ * @param action {String} One of 'watch', 'unwatch'.
 14+ * @param state {String} [optional] 'idle' or 'loading'. Default is 'idle'.
 15+ */
 16+function updateWatchLink( $link, action, state ) {
 17+ // message keys 'watch', 'watching', 'unwatch' or 'unwatching'.
 18+ var msgKey = state === 'loading' ? action + 'ing' : action,
 19+ accesskeyTip = $link.attr( 'title' ).match( mw.util.tooltipAccessKeyRegexp ),
 20+ $li = $link.closest( 'li' );
 21+
 22+ $link
 23+ .text( mw.msg( msgKey ) )
 24+ .attr( 'title', mw.msg( 'tooltip-ca-' + action ) +
 25+ ( accesskeyTip ? ' ' + accesskeyTip[0] : '' )
 26+ )
 27+ .attr( 'href', mw.util.wikiScript() + '?' + $.param({
 28+ title: mw.config.get( 'wgPageName' ),
 29+ action: action
 30+ })
 31+ );
 32+
 33+ // Special case for vector icon
 34+ if ( $li.hasClass( 'icon' ) ) {
 35+ if ( state === 'loading' ) {
1836 $link.addClass( 'loading' );
1937 } else {
2038 $link.removeClass( 'loading' );
2139 }
22 - } else {
23 - $link.html( mw.msg( action ) );
2440 }
25 -};
 41+}
2642
27 -var errorHandler = function( $link ) {
28 -
29 - // Reset link text to whatever it was before we switching it to the '(un)watch'+ing message.
30 - setLinkText( $link, $link.data( 'action' ) );
31 -
32 - // Format error message
33 - var cleanTitle = mw.config.get( 'wgPageName' ).replace( /_/g, ' ' );
34 - var link = mw.html.element(
35 - 'a', {
36 - 'href': mw.util.wikiGetlink( mw.config.get( 'wgPageName' ) ),
37 - 'title': cleanTitle
38 - }, cleanTitle
39 - );
40 - var msg = mw.msg( 'watcherrortext', link );
41 -
42 - // Report to user about the error
43 - mw.util.jsMessage( msg, 'watch' );
44 -};
45 -
4643 /**
47 - * Process the result of the API watch action.
48 - *
49 - * @param response Data object from API request.
50 - * @param $link jQuery object of the watch link.
51 - * @return Boolean true on success, false otherwise.
 44+ * @todo This should be moved somewhere more accessible.
 45+ * @param url {String}
 46+ * @return {String} The extracted action, defaults to 'view'.
5247 */
53 -var processResult = function( response, $link ) {
 48+function mwUriGetAction( url ) {
 49+ var actionPaths = mw.config.get( 'wgActionPaths' ),
 50+ key, parts, m, action;
5451
55 - if ( ( 'error' in response ) || !response.watch ) {
56 - errorHandler( $link );
57 - return false;
 52+ // @todo: Does MediaWiki give action path or query param
 53+ // precedence ? If the former, move this to the bottom
 54+ action = mw.util.getParamValue( 'action', url );
 55+ if ( action !== null ) {
 56+ return action;
5857 }
5958
60 - var watchResponse = response.watch;
61 -
62 - // To ensure we set the same status for all watch links with the
63 - // same target we trigger a custom event on *all* watch links.
64 - if ( watchResponse.watched !== undefined ) {
65 - $links.trigger( 'mw-ajaxwatch', [watchResponse.title, 'watch', $link] );
66 - } else if ( watchResponse.unwatched !== undefined ) {
67 - $links.trigger( 'mw-ajaxwatch', [watchResponse.title, 'unwatch', $link] );
68 - } else {
69 - // Either we got an error code or it just plain broke.
70 - window.location.href = $link[0].href;
71 - return false;
 59+ for ( key in actionPaths ) {
 60+ if ( actionPaths.hasOwnProperty( key ) ) {
 61+ parts = actionPaths[key].split( '$1' );
 62+ for ( i = 0; i < parts.length; i += 1 ) {
 63+ parts[i] = $.escapeRE( parts[i] );
 64+ }
 65+ m = new RegExp( parts.join( '(.+)' ) ).exec( url );
 66+ if ( m && m[1] ) {
 67+ return key;
 68+ }
 69+
 70+ }
7271 }
7372
74 - mw.util.jsMessage( watchResponse.message, 'watch' );
 73+ return 'view';
 74+}
7575
76 - // Bug 12395 - update the watch checkbox on edit pages when the
77 - // page is watched or unwatched via the tab.
78 - if ( watchResponse.watched !== undefined ) {
79 - $( '#wpWatchthis' ).prop( 'checked', 'checked' );
80 - } else {
81 - $( '#wpWatchthis' ).removeProp( 'checked' );
82 - }
83 - return true;
84 -};
85 -
8676 $( document ).ready( function() {
87 - $links = $( '.mw-watchlink a, a.mw-watchlink' );
88 - // BC with older skins
89 - $links = $links
90 - .add( '#ca-watch a, #ca-unwatch a, a#mw-unwatch-link1, ' +
91 - 'a#mw-unwatch-link2, a#mw-watch-link2, a#mw-watch-link1' );
92 - // allowing people to add inline animated links is a little scary
 77+ var $links = $( '.mw-watchlink a, a.mw-watchlink' +
 78+ '#ca-watch a, #ca-unwatch a, #mw-unwatch-link1, ' +
 79+ '#mw-unwatch-link2, #mw-watch-link2, #mw-watch-link1' );
 80+
 81+ // Allowing people to add inline animated links is a little scary
9382 $links = $links.filter( ':not( #bodyContent *, #content * )' );
9483
95 - $links.each( function() {
96 - var $link = $( this );
97 - var link = this;
98 - $link
99 - .data( 'icon', $link.closest( 'li' ).hasClass( 'icon' ) )
100 - .data( 'action', mw.util.getParamValue( 'action', link.href ) == 'unwatch' ? 'unwatch' : 'watch' );
101 - var title = mw.util.getParamValue( 'title', link.href );
102 - $link.data( 'target', title.replace( /_/g, ' ' ) );
103 - });
 84+ $links.click( function( e ) {
 85+ var $link, api,
 86+ action = mwUriGetAction( this.href );
10487
105 - $links.click( function( event ) {
106 - var $link = $( this );
107 -
108 - if ( !mw.config.get( 'wgEnableWriteAPI' ) ) {
109 - // Lazy initialization so we don't toss up
110 - // ActiveX warnings on initial page load
111 - // for IE 6 users with security settings.
112 - $links.unbind( 'click' );
 88+ if ( action !== 'watch' && action !== 'unwatch' ) {
 89+ // Could not extract target action from link url,
 90+ // let native browsing handle it further
11391 return true;
11492 }
 93+ e.preventDefault();
 94+ e.stopPropagation();
 95+
 96+ $link = $( this );
11597
116 - setLinkText( $link, $link.data( 'action' ) + 'ing' );
 98+ updateWatchLink( $link, action, 'loading' );
11799
118 - var reqData = {
119 - 'action': 'watch',
120 - 'format': 'json',
121 - 'title': $link.data( 'target' ),
122 - 'token': mw.user.tokens.get( 'watchToken' ),
123 - // API return contains a localized data.watch.message string.
124 - 'uselang': mw.config.get( 'wgUserLanguage' )
125 - };
 100+ api = new mw.Api();
 101+ api[action](
 102+ mw.config.get( 'wgPageName' ),
 103+ // Success
 104+ function( watchResponse ) {
 105+ var otherAction = action === 'watch' ? 'unwatch' : 'watch',
 106+ $li = $link.closest( 'li' );
126107
127 - if ( $link.data( 'action' ) == 'unwatch' ) {
128 - reqData.unwatch = '';
129 - }
 108+ mw.util.jsMessage( watchResponse.message, 'ajaxwatch' );
130109
131 - $.ajax({
132 - url: mw.util.wikiScript( 'api' ),
133 - dataType: 'json',
134 - type: 'POST',
135 - data: reqData,
136 - success: function( data, textStatus, xhr ) {
137 - processResult( data, $link );
 110+ // Set link to opposite
 111+ updateWatchLink( $link, otherAction );
 112+
 113+ // Most common ID style
 114+ if ( $li.prop( 'id' ) === 'ca-' + otherAction || $li.prop( 'id' ) === 'ca-' + action ) {
 115+ $li.prop( 'id', 'ca-' + otherAction );
 116+ }
 117+
 118+ // Bug 12395 - update the watch checkbox on edit pages when the
 119+ // page is watched or unwatched via the tab.
 120+ if ( watchResponse.watched !== undefined ) {
 121+ $( '#wpWatchthis' ).prop( 'checked', true );
 122+ } else {
 123+ $( '#wpWatchthis' ).removeProp( 'checked' );
 124+ }
138125 },
139 - error: function(){
140 - processResult( {}, $link );
141 - }
142 - });
 126+ // Error
 127+ function(){
143128
144 - return false;
145 - });
 129+ // Reset link to non-loading mode
 130+ updateWatchLink( $link, action );
 131+
 132+ // Format error message
 133+ var cleanTitle = mw.config.get( 'wgPageName' ).replace( /_/g, ' ' );
 134+ var link = mw.html.element(
 135+ 'a', {
 136+ 'href': mw.util.wikiGetlink( mw.config.get( 'wgPageName' ) ),
 137+ 'title': cleanTitle
 138+ }, cleanTitle
 139+ );
 140+ var html = mw.msg( 'watcherrortext', link );
 141+
 142+ // Report to user about the error
 143+ mw.util.jsMessage( html, 'ajaxwatch' );
146144
147 - // When a request returns, a custom event 'mw-ajaxwatch' is triggered
148 - // on *all* watch links, so they can be updated if necessary
149 - $links.bind( 'mw-ajaxwatch', function( event, target, action, $link ) {
150 - var foo = $link.data( 'target' );
151 - if ( $link.data( 'target' ) == target ) {
152 - var otheraction = action == 'watch'
153 - ? 'unwatch'
154 - : 'watch';
155 -
156 - $link.data( 'action', otheraction );
157 - setLinkText( $link, otheraction );
158 - $link.attr( 'href',
159 - mw.config.get( 'wgScript' )
160 - + '?title=' + mw.util.wikiUrlencode( mw.config.get( 'wgPageName' ) )
161 - + '&action=' + otheraction
162 - );
163 - if ( $link.closest( 'li' ).attr( 'id' ) == 'ca-' + action ) {
164 - $link.closest( 'li' ).attr( 'id', 'ca-' + otheraction );
165 - // update the link text with the new message
166 - $link.text( mw.msg( otheraction ) );
167145 }
168 - }
169 -
170 - return false;
 146+ );
171147 });
172148
173149 });
174150
175 -})( jQuery );
 151+})( jQuery, mediaWiki );
Index: trunk/phase3/resources/mediawiki/mediawiki.util.js
@@ -413,10 +413,10 @@
414414 * something, replacing any previous message.
415415 * Calling with no arguments, with an empty string or null will hide the message
416416 *
417 - * @param message mixed The DOM-element or HTML-string to be put inside the message box.
418 - * @param className string Used in adding a class; should be different for each call
 417+ * @param message {mixed} The DOM-element, jQuery object or HTML-string to be put inside the message box.
 418+ * @param className {String} Used in adding a class; should be different for each call
419419 * to allow CSS/JS to hide different boxes. null = no class used.
420 - * @return boolean True on success, false on failure.
 420+ * @return {Boolean} True on success, false on failure.
421421 */
422422 jsMessage: function( message, className ) {
423423 if ( !arguments.length || message === '' || message === null ) {
@@ -443,7 +443,7 @@
444444
445445 if ( typeof message === 'object' ) {
446446 $messageDiv.empty();
447 - $messageDiv.append( message ); // Append new content
 447+ $messageDiv.append( message );
448448 } else {
449449 $messageDiv.html( message );
450450 }
Index: trunk/phase3/resources/mediawiki.page/mediawiki.page.startup.js
@@ -2,9 +2,8 @@
33
44 mw.page = {};
55
6 - /* Client profile classes for <html> */
7 - /* Allows for easy hiding/showing of JS or no-JS-specific UI elements */
8 -
 6+ // Client profile classes for <html>
 7+ // Allows for easy hiding/showing of JS or no-JS-specific UI elements
98 $( 'html' )
109 .addClass('client-js' )
1110 .removeClass( 'client-nojs' );

Follow-up revisions

RevisionCommit summaryAuthorDate
r107963Fix regression in r107354: typo caused 'watch' link in Vector to never be hoo...brion23:41, 3 January 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r78147start resourceloadify mw.legacy.ajaxwatchkrinkle21:59, 9 December 2010
r78150Fixing bugs and modernising mediawiki.action.watch.ajax.js a little bit...krinkle22:45, 9 December 2010
r82498bug 27146krinkle00:12, 20 February 2011
r88511Removing wgAjaxWatch javascript global object....krinkle11:06, 21 May 2011
r88527Fixing mediawiki.action.watch.ajax...krinkle18:33, 21 May 2011
r107350[mediawiki.api] write mediawiki.api.watch module...krinkle00:44, 27 December 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   23:28, 3 January 2012

Ajax watch seems broken for me; clicking the watch tab just sends me on through to the action URL for watching, which fails because I faked it.

#Comment by Brion VIBBER (talk | contribs)   23:42, 3 January 2012

Fixed in r107963 -- a comma and space got left out when tweaking the selector lines.

#Comment by IAlex (talk | contribs)   17:25, 13 March 2012

This causes bug 34972 since wgPageName is not the correct one on special pages.

#Comment by IAlex (talk | contribs)   18:18, 13 March 2012

Fixed in r113737.

Status & tagging log