r107011 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r107010‎ | r107011 | r107012 >
Date:23:52, 21 December 2011
Author:krinkle
Status:ok (Comments)
Tags:
Comment:
[mediawiki.api] clean up

* Adding return values to most of the ajax functions so that the jqXHR object (originally returned by jQuery.ajax) is available. Some functions documented they already did this, some don't. Now they all do and are also documented as such.

* Renaming a few 'callback' arguments to 'success' to avoid confusion with 'error'.

* Removed unused variables
** mw.Api's instance this.url was unused
** var cachedToken was unused in mediawiki.api.titleblacklist.js

* Making closure argument order the same and referencing mediaWiki as a global (mw is something thought not to be a global but mediaWiki certainly is, no need to access window from the global scope and then the mediaWiki property of it). Also renamed any used of $j to $. Adding 'undefined' to the closure arguments where missing.

* Re-written the mw.Api constructor's logic for options. Now keeping a defaultOptions privately "statically" cached outside the constructor and using that to build the options object.

* Made all non-block comments the same comment style (some were /* */ or /* \n */)

* Completed parameter documentation and wrote TODO as @todo and added an example use for mw.Api in the documentation.

* Some other random whitespacing, line breaking, merged var statement, and moved them out of blocks (since JS doesn't have block scope) into the main function body for clarity. And some other random JS Lint/JS Hint stuff.

* Added comment to api.titleblacklist module about the module not being in core but in the TitleBlacklist extension.
Modified paths:
  • /trunk/phase3/resources/mediawiki/mediawiki.api.category.js (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.api.edit.js (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.api.js (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.api.parse.js (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.api.titleblacklist.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki/mediawiki.api.js
——@@ -1,179 +1,189 @@
2 -/* mw.Api objects represent the API of a particular MediaWiki server. */
 2+/* mw.Api objects represent the API of a particular MediaWiki server. */
33
4 -( function( mw, $j, undefined ) {
5 -
 4+( function( $, mw, undefined ) {
 5+
66 /**
7 - * Represents the API of a particular MediaWiki server.
8 - *
9 - * Required options:
10 - * url - complete URL to API endpoint. Usually equivalent to wgServer + wgScriptPath + '/api.php'
11 - *
12 - * Other options:
13 - * can override the parameter defaults and ajax default options.
14 - * XXX document!
15 - *
16 - * TODO share api objects with exact same config.
17 - *
18 - * ajax options can also be overriden on every get() or post()
19 - *
20 - * @param options {Mixed} can take many options, but must include at minimum the API url.
 7+ * @var defaultsOptions {Object}
 8+ * We allow people to omit these default parameters from API requests
 9+ * there is very customizable error handling here, on a per-call basis
 10+ * wondering, would it be simpler to make it easy to clone the api object,
 11+ * change error handling, and use that instead?
2112 */
22 - mw.Api = function( options ) {
 13+ var defaultsOptions = {
2314
24 - if ( options === undefined ) {
25 - options = {};
26 - }
27 -
28 - // make sure we at least have a URL endpoint for the API
29 - if ( options.url === undefined ) {
30 - options.url = mw.config.get( 'wgServer' ) + mw.config.get( 'wgScriptPath' ) + '/api' + mw.config.get( 'wgScriptExtension' );
31 - }
32 -
33 - this.url = options.url;
34 -
35 - /* We allow people to omit these default parameters from API requests */
36 - // there is very customizable error handling here, on a per-call basis
37 - // wondering, would it be simpler to make it easy to clone the api object, change error handling, and use that instead?
38 - this.defaults = {
 15+ // Query parameters for API requests
3916 parameters: {
4017 action: 'query',
4118 format: 'json'
4219 },
4320
 21+ // Ajax options for jQuery.ajax()
4422 ajax: {
45 - // force toString if we got a mw.Uri object
46 - url: new String( this.url ),
 23+ url: mw.util.wikiScript( 'api' ),
4724
48 - /* default function for success and no API error */
4925 ok: function() {},
5026
5127 // caller can supply handlers for http transport error or api errors
5228 err: function( code, result ) {
53 - mw.log( "mw.Api error: " + code, 'debug' );
 29+ mw.log( 'mw.Api error: ' + code, 'debug' );
5430 },
5531
56 - timeout: 30000, /* 30 seconds */
 32+ timeout: 30000, // 30 seconds
5733
5834 dataType: 'json'
59 -
6035 }
6136 };
6237
 38+ /**
 39+ * Constructor to create an object to interact with the API of a particular MediaWiki server.
 40+ *
 41+ * @todo Share API objects with exact same config.
 42+ * @example
 43+ * <code>
 44+ * var api = new mw.Api();
 45+ * api.get( {
 46+ * action: 'query',
 47+ * meta: 'userinfo'
 48+ * }, {
 49+ * ok: function () { console.log( arguments ); }
 50+ * } );
 51+ * </code>
 52+ *
 53+ * @constructor
 54+ * @param options {Object} See defaultOptions documentation above. Ajax options can also be
 55+ * overridden for each individual request to jQuery.ajax() later on.
 56+ */
 57+ mw.Api = function( options ) {
6358
64 - if ( options.parameters ) {
65 - $j.extend( this.defaults.parameters, options.parameters );
 59+ if ( options === undefined ) {
 60+ options = {};
6661 }
6762
68 - if ( options.ajax ) {
69 - $j.extend( this.defaults.ajax, options.ajax );
 63+ // Force toString if we got a mw.Uri object
 64+ if ( options.ajax && options.ajax.url !== undefined ) {
 65+ options.ajax.url = String( options.ajax.url );
7066 }
 67+
 68+ options.parameters = $.extend( {}, defaultsOptions.parameters, options.parameters );
 69+ options.ajax = $.extend( {}, defaultsOptions.ajax, options.ajax );
 70+
 71+ this.defaults = options;
7172 };
7273
7374 mw.Api.prototype = {
7475
7576 /**
7677 * For api queries, in simple cases the caller just passes a success callback.
77 - * In complex cases they pass an object with a success property as callback and probably other options.
 78+ * In complex cases they pass an object with a success property as callback and
 79+ * probably other options.
7880 * Normalize the argument so that it's always the latter case.
79 - *
80 - * @param {Object|Function} ajax properties, or just a success function
81 - * @return Function
 81+ *
 82+ * @param {Object|Function} An object contaning one or more of options.ajax,
 83+ * or just a success function (options.ajax.ok).
 84+ * @return {Object} Normalized ajax options.
8285 */
8386 normalizeAjaxOptions: function( arg ) {
 87+ var opt = arg;
8488 if ( typeof arg === 'function' ) {
85 - var ok = arg;
86 - arg = { 'ok': ok };
 89+ opt = { 'ok': arg };
8790 }
88 - if (! arg.ok ) {
89 - throw Error( "ajax options must include ok callback" );
 91+ if ( !opt.ok ) {
 92+ throw new Error( 'ajax options must include ok callback' );
9093 }
91 - return arg;
 94+ return opt;
9295 },
9396
9497 /**
9598 * Perform API get request
9699 *
97 - * @param {Object} request parameters
98 - * @param {Object|Function} ajax properties, or just a success function
99 - */
 100+ * @param {Object} request parameters
 101+ * @param {Object|Function} ajax options, or just a success function
 102+ * @return {jqXHR}
 103+ */
100104 get: function( parameters, ajaxOptions ) {
101105 ajaxOptions = this.normalizeAjaxOptions( ajaxOptions );
102106 ajaxOptions.type = 'GET';
103 - this.ajax( parameters, ajaxOptions );
 107+ return this.ajax( parameters, ajaxOptions );
104108 },
105109
106110 /**
107111 * Perform API post request
108 - * TODO post actions for nonlocal will need proxy
109 - *
110 - * @param {Object} request parameters
111 - * @param {Object|Function} ajax properties, or just a success function
 112+ * @todo Post actions for nonlocal will need proxy
 113+ *
 114+ * @param {Object} request parameters
 115+ * @param {Object|Function} ajax options, or just a success function
 116+ * @return {jqXHR}
112117 */
113118 post: function( parameters, ajaxOptions ) {
114119 ajaxOptions = this.normalizeAjaxOptions( ajaxOptions );
115120 ajaxOptions.type = 'POST';
116 - this.ajax( parameters, ajaxOptions );
 121+ return this.ajax( parameters, ajaxOptions );
117122 },
118123
119124 /**
120 - * Perform the API call.
121 - *
122 - * @param {Object} request parameters
123 - * @param {Object} ajax properties
 125+ * Perform the API call.
 126+ *
 127+ * @param {Object} request parameters
 128+ * @param {Object} ajax options
 129+ * @return {jqXHR}
124130 */
125131 ajax: function( parameters, ajaxOptions ) {
126 - parameters = $j.extend( {}, this.defaults.parameters, parameters );
127 - ajaxOptions = $j.extend( {}, this.defaults.ajax, ajaxOptions );
 132+ parameters = $.extend( {}, this.defaults.parameters, parameters );
 133+ ajaxOptions = $.extend( {}, this.defaults.ajax, ajaxOptions );
128134
129135 // Some deployed MediaWiki >= 1.17 forbid periods in URLs, due to an IE XSS bug
130136 // So let's escape them here. See bug #28235
131137 // This works because jQuery accepts data as a query string or as an Object
132 - ajaxOptions.data = $j.param( parameters ).replace( /\./g, '%2E' );
133 -
 138+ ajaxOptions.data = $.param( parameters ).replace( /\./g, '%2E' );
 139+
134140 ajaxOptions.error = function( xhr, textStatus, exception ) {
135 - ajaxOptions.err( 'http', { xhr: xhr, textStatus: textStatus, exception: exception } );
 141+ ajaxOptions.err( 'http', {
 142+ xhr: xhr,
 143+ textStatus: textStatus,
 144+ exception: exception
 145+ } );
136146 };
137147
138 -
139 - /* success just means 200 OK; also check for output and API errors */
 148+ // Success just means 200 OK; also check for output and API errors
140149 ajaxOptions.success = function( result ) {
141150 if ( result === undefined || result === null || result === '' ) {
142 - ajaxOptions.err( "ok-but-empty", "OK response but empty result (check HTTP headers?)" );
 151+ ajaxOptions.err( 'ok-but-empty',
 152+ 'OK response but empty result (check HTTP headers?)' );
143153 } else if ( result.error ) {
144154 var code = result.error.code === undefined ? 'unknown' : result.error.code;
145155 ajaxOptions.err( code, result );
146 - } else {
 156+ } else {
147157 ajaxOptions.ok( result );
148158 }
149159 };
150160
151 - $j.ajax( ajaxOptions );
152 -
 161+ return $.ajax( ajaxOptions );
153162 }
154163
155164 };
156165
157166 /**
158 - * This is a list of errors we might receive from the API.
 167+ * @var {Array} List of errors we might receive from the API.
159168 * For now, this just documents our expectation that there should be similar messages
160169 * available.
161170 */
162171 mw.Api.errors = [
163 - /* occurs when POST aborted - jQuery 1.4 can't distinguish abort or lost connection from 200 OK + empty result */
 172+ // occurs when POST aborted
 173+ // jQuery 1.4 can't distinguish abort or lost connection from 200 OK + empty result
164174 'ok-but-empty',
165175
166176 // timeout
167177 'timeout',
168178
169 - /* really a warning, but we treat it like an error */
 179+ // really a warning, but we treat it like an error
170180 'duplicate',
171181 'duplicate-archive',
172182
173 - /* upload succeeded, but no image info.
174 - this is probably impossible, but might as well check for it */
 183+ // upload succeeded, but no image info.
 184+ // this is probably impossible, but might as well check for it
175185 'noimageinfo',
176186
177 - /* remote errors, defined in API */
 187+ // remote errors, defined in API
178188 'uploaddisabled',
179189 'nomodule',
180190 'mustbeposted',
——@@ -201,14 +211,13 @@
202212 ];
203213
204214 /**
205 - * This is a list of warnings we might receive from the API.
 215+ * @var {Array} List of warnings we might receive from the API.
206216 * For now, this just documents our expectation that there should be similar messages
207217 * available.
208218 */
209 -
210219 mw.Api.warnings = [
211220 'duplicate',
212221 'exists'
213222 ];
214223
215 -}) ( window.mediaWiki, jQuery );
 224+})( jQuery, mediaWiki );
Index: trunk/phase3/resources/mediawiki/mediawiki.api.parse.js
——@@ -1,29 +1,31 @@
2 -// library to assist with action=parse, that is, get rendered HTML of wikitext
 2+/**
 3+ * Additional mw.Api methods to assist with API calls related to parsing wikitext.
 4+ */
35
4 -( function( mw, $ ) {
 6+( function( $, mw ) {
57
6 - $.extend( mw.Api.prototype, {
 8+ $.extend( mw.Api.prototype, {
79 /**
8 - * Parse wikitext into HTML
9 - * @param {String} wikitext
10 - * @param {Function} callback to which to pass success HTML
11 - * @param {Function} callback if error (optional)
 10+ * Convinience method for 'action=parse'. Parses wikitext into HTML.
 11+ *
 12+ * @param wikiText {String}
 13+ * @param success {Function} callback to which to pass success HTML
 14+ * @param error {Function} callback if error (optional)
 15+ * @return {jqXHR}
1216 */
13 - parse: function( wikiText, useHtml, error ) {
 17+ parse: function( wikiText, success, error ) {
1418 var params = {
15 - text: wikiText,
16 - action: 'parse'
17 - };
18 - var ok = function( data ) {
19 - if ( data && data.parse && data.parse.text && data.parse.text['*'] ) {
20 - useHtml( data.parse.text['*'] );
21 - }
22 - };
23 - this.get( params, ok, error );
 19+ text: wikiText,
 20+ action: 'parse'
 21+ },
 22+ ok = function( data ) {
 23+ if ( data && data.parse && data.parse.text && data.parse.text['*'] ) {
 24+ success( data.parse.text['*'] );
 25+ }
 26+ };
 27+ return this.get( params, ok, error );
2428 }
2529
 30+ } );
2631
27 - } ); // end extend
28 -} )( window.mediaWiki, jQuery );
29 -
30 -
 32+} )( jQuery, mediaWiki );
Index: trunk/phase3/resources/mediawiki/mediawiki.api.titleblacklist.js
——@@ -1,48 +1,51 @@
2 -// library to assist with API calls on titleblacklist
 2+/**
 3+ * Additional mw.Api methods to assist with API calls to the API module of the TitleBlacklist extension.
 4+ */
35
4 -( function( mw, $ ) {
 6+( function( $, mw, undefined ) {
57
6 - // cached token so we don't have to keep fetching new ones for every single post
7 - var cachedToken = null;
8 -
9 - $.extend( mw.Api.prototype, {
 8+ $.extend( mw.Api.prototype, {
109 /**
11 - * @param {mw.Title}
12 - * @param {Function} callback to pass false on Title not blacklisted, or error text when blacklisted
13 - * @param {Function} optional callback to run if api error
14 - * @return ajax call object
 10+ * Convinience method for 'action=titleblacklist'.
 11+ * Note: This action is not provided by MediaWiki core, but as part of the TitleBlacklist extension.
 12+ *
 13+ * @param title {mw.Title}
 14+ * @param success {Function} Called on successfull request. First argument is false if title wasn't blacklisted,
 15+ * object with 'reason', 'line' and 'message' properties if title was blacklisted.
 16+ * @param err {Function} optional callback to run if api error
 17+ * @return {jqXHR}
1518 */
16 - isBlacklisted: function( title, callback, err ) {
17 - var params = {
18 - 'action': 'titleblacklist',
19 - 'tbaction': 'create',
20 - 'tbtitle': title.toString()
21 - };
 19+ isBlacklisted: function( title, success, err ) {
 20+ var params = {
 21+ action: 'titleblacklist',
 22+ tbaction: 'create',
 23+ tbtitle: title.toString()
 24+ },
 25+ ok = function( data ) {
 26+ var result;
2227
23 - var ok = function( data ) {
24 - // this fails open (if nothing valid is returned by the api, allows the title)
25 - // also fails open when the API is not present, which will be most of the time.
26 - if ( data.titleblacklist && data.titleblacklist.result && data.titleblacklist.result == 'blacklisted') {
27 - var result;
28 - if ( data.titleblacklist.reason ) {
29 - result = {
30 - reason: data.titleblacklist.reason,
31 - line: data.titleblacklist.line,
32 - message: data.titleblacklist.message
33 - };
 28+ // this fails open (if nothing valid is returned by the api, allows the title)
 29+ // also fails open when the API is not present, which will be most of the time
 30+ // as this API module is part of the TitleBlacklist extension.
 31+ if ( data.titleblacklist && data.titleblacklist.result && data.titleblacklist.result === 'blacklisted') {
 32+ if ( data.titleblacklist.reason ) {
 33+ result = {
 34+ reason: data.titleblacklist.reason,
 35+ line: data.titleblacklist.line,
 36+ message: data.titleblacklist.message
 37+ };
 38+ } else {
 39+ mw.log('mw.Api.titleblacklist::isBlacklisted> no reason data for blacklisted title', 'debug');
 40+ result = { reason: 'Blacklisted, but no reason supplied', line: 'Unknown', message: null };
 41+ }
 42+ success( result );
3443 } else {
35 - mw.log("mw.Api.titleblacklist::isBlacklisted> no reason data for blacklisted title", 'debug');
36 - result = { reason: "Blacklisted, but no reason supplied", line: "Unknown" };
 44+ success ( false );
3745 }
38 - callback( result );
39 - } else {
40 - callback ( false );
41 - }
42 - };
 46+ };
4347
4448 return this.get( params, ok, err );
45 -
4649 }
4750
4851 } );
49 -} )( window.mediaWiki, jQuery );
 52+} )( jQuery, mediaWiki );
Index: trunk/phase3/resources/mediawiki/mediawiki.api.category.js
——@@ -1,46 +1,46 @@
2 -// library to assist with API calls on categories
 2+/**
 3+ * Additional mw.Api methods to assist with API calls related to categories.
 4+ */
35
4 -( function( mw, $ ) {
 6+( function( $, mw, undefined ) {
57
6 - $.extend( mw.Api.prototype, {
 8+ $.extend( mw.Api.prototype, {
79 /**
8 - * Determine if a category exists
9 - * @param {mw.Title}
10 - * @param {Function} callback to pass boolean of category's existence
11 - * @param {Function} optional callback to run if api error
 10+ * Determine if a category exists.
 11+ * @param title {mw.Title}
 12+ * @param success {Function} callback to pass boolean of category's existence
 13+ * @param err {Function} optional callback to run if api error
1214 * @return ajax call object
1315 */
14 - isCategory: function( title, callback, err ) {
 16+ isCategory: function( title, success, err ) {
1517 var params = {
16 - 'prop': 'categoryinfo',
17 - 'titles': title.toString()
18 - };
 18+ prop: 'categoryinfo',
 19+ titles: title.toString()
 20+ },
 21+ ok = function( data ) {
 22+ var exists = false;
 23+ if ( data.query && data.query.pages ) {
 24+ $.each( data.query.pages, function( id, page ) {
 25+ if ( page.categoryinfo ) {
 26+ exists = true;
 27+ }
 28+ } );
 29+ }
 30+ success( exists );
 31+ };
1932
20 - var ok = function( data ) {
21 - var exists = false;
22 - if ( data.query && data.query.pages ) {
23 - $.each( data.query.pages, function( id, page ) {
24 - if ( page.categoryinfo ) {
25 - exists = true;
26 - }
27 - } );
28 - }
29 - callback( exists );
30 - };
31 -
3233 return this.get( params, { ok: ok, err: err } );
33 -
3434 },
3535
3636 /**
37 - * Get a list of categories that match a certain prefix.
 37+ * Get a list of categories that match a certain prefix.
3838 * e.g. given "Foo", return "Food", "Foolish people", "Foosball tables" ...
39 - * @param {String} prefix to match
40 - * @param {Function} callback to pass matched categories to
41 - * @param {Function} optional callback to run if api error
42 - * @return ajax call object
 39+ * @param prefix {String} prefix to match
 40+ * @param success {Function} callback to pass matched categories to
 41+ * @param err {Function} optional callback to run if api error
 42+ * @return {jqXHR}
4343 */
44 - getCategoriesByPrefix: function( prefix, callback, err ) {
 44+ getCategoriesByPrefix: function( prefix, success, err ) {
4545
4646 // fetch with allpages to only get categories that have a corresponding description page.
4747 var params = {
——@@ -51,56 +51,55 @@
5252
5353 var ok = function( data ) {
5454 var texts = [];
55 - if ( data.query && data.query.allpages ) {
 55+ if ( data.query && data.query.allpages ) {
5656 $.each( data.query.allpages, function( i, category ) {
57 - texts.push( new mw.Title( category.title ).getNameText() );
 57+ texts.push( new mw.Title( category.title ).getNameText() );
5858 } );
5959 }
60 - callback( texts );
 60+ success( texts );
6161 };
6262
6363 return this.get( params, { ok: ok, err: err } );
64 -
6564 },
6665
6766
6867 /**
6968 * Get the categories that a particular page on the wiki belongs to
70 - * @param {mw.Title}
71 - * @param {Function} callback to pass categories to (or false, if title not found)
72 - * @param {Function} optional callback to run if api error
73 - * @param {Boolean} optional asynchronousness (default = true = async)
74 - * @return ajax call object
 69+ * @param title {mw.Title}
 70+ * @param success {Function} callback to pass categories to (or false, if title not found)
 71+ * @param err {Function} optional callback to run if api error
 72+ * @param async {Boolean} optional asynchronousness (default = true = async)
 73+ * @return {jqXHR}
7574 */
76 - getCategories: function( title, callback, err, async ) {
77 - var params = {
 75+ getCategories: function( title, success, err, async ) {
 76+ var params, ok;
 77+ params = {
7878 prop: 'categories',
7979 titles: title.toString()
8080 };
8181 if ( async === undefined ) {
8282 async = true;
8383 }
84 -
85 - var ok = function( data ) {
 84+ ok = function( data ) {
8685 var ret = false;
8786 if ( data.query && data.query.pages ) {
8887 $.each( data.query.pages, function( id, page ) {
8988 if ( page.categories ) {
90 - if ( typeof ret !== 'object' ) {
 89+ if ( typeof ret !== 'object' ) {
9190 ret = [];
9291 }
93 - $.each( page.categories, function( i, cat ) {
94 - ret.push( new mw.Title( cat.title ) );
 92+ $.each( page.categories, function( i, cat ) {
 93+ ret.push( new mw.Title( cat.title ) );
9594 } );
9695 }
9796 } );
9897 }
99 - callback( ret );
 98+ success( ret );
10099 };
101100
102101 return this.get( params, { ok: ok, err: err, async: async } );
103 -
104102 }
105103
106104 } );
107 -} )( window.mediaWiki, jQuery );
 105+
 106+} )( jQuery, mediaWiki );
Index: trunk/phase3/resources/mediawiki/mediawiki.api.edit.js
——@@ -1,104 +1,106 @@
2 -// library to assist with edits
 2+/**
 3+ * Additional mw.Api methods to assist with API calls related to editing wiki pages.
 4+ */
35
4 -( function( mw, $, undefined ) {
 6+( function( $, mw, undefined ) {
57
6 - // cached token so we don't have to keep fetching new ones for every single post
 8+ // Cache token so we don't have to keep fetching new ones for every single request.
79 var cachedToken = null;
810
9 - $.extend( mw.Api.prototype, {
 11+ $.extend( mw.Api.prototype, {
1012
11 - /* Post to API with edit token. If we have no token, get one and try to post.
12 - * If we have a cached token try using that, and if it fails, blank out the
13 - * cached token and start over.
14 - *
15 - * @param params API parameters
16 - * @param ok callback for success
17 - * @param err (optional) error callback
 13+ /**
 14+ * Post to API with edit token. If we have no token, get one and try to post.
 15+ * If we have a cached token try using that, and if it fails, blank out the
 16+ * cached token and start over.
 17+ *
 18+ * @param params {Object} API parameters
 19+ * @param ok {Function} callback for success
 20+ * @param err {Function} [optional] error callback
 21+ * @return {jqXHR}
1822 */
1923 postWithEditToken: function( params, ok, err ) {
20 - var api = this;
 24+ var api = this, useTokenToPost, getTokenIfBad;
2125 if ( cachedToken === null ) {
2226 // We don't have a valid cached token, so get a fresh one and try posting.
2327 // We do not trap any 'badtoken' or 'notoken' errors, because we don't want
2428 // an infinite loop. If this fresh token is bad, something else is very wrong.
25 - var useTokenToPost = function( token ) {
26 - params.token = token;
 29+ useTokenToPost = function( token ) {
 30+ params.token = token;
2731 api.post( params, ok, err );
2832 };
29 - api.getEditToken( useTokenToPost, err );
 33+ return api.getEditToken( useTokenToPost, err );
3034 } else {
3135 // We do have a token, but it might be expired. So if it is 'bad' then
3236 // start over with a new token.
3337 params.token = cachedToken;
34 - var getTokenIfBad = function( code, result ) {
35 - if ( code === 'badtoken' ) {
 38+ getTokenIfBad = function( code, result ) {
 39+ if ( code === 'badtoken' ) {
3640 cachedToken = null; // force a new token
3741 api.postWithEditToken( params, ok, err );
3842 } else {
3943 err( code, result );
4044 }
4145 };
42 - api.post( params, { 'ok' : ok, 'err' : getTokenIfBad });
 46+ return api.post( params, { 'ok' : ok, 'err' : getTokenIfBad });
4347 }
4448 },
45 -
 49+
4650 /**
4751 * Api helper to grab an edit token
48 - *
 52+ *
4953 * token callback has signature ( String token )
5054 * error callback has signature ( String code, Object results, XmlHttpRequest xhr, Exception exception )
51 - * Note that xhr and exception are only available for 'http_*' errors
 55+ * Note that xhr and exception are only available for 'http_*' errors
5256 * code may be any http_* error code (see mw.Api), or 'token_missing'
5357 *
54 - * @param {Function} received token callback
55 - * @param {Function} error callback
 58+ * @param tokenCallback {Function} received token callback
 59+ * @param err {Function} error callback
 60+ * @return {jqXHR}
5661 */
5762 getEditToken: function( tokenCallback, err ) {
58 - var api = this;
59 -
60 - var parameters = {
61 - 'prop': 'info',
62 - 'intoken': 'edit',
63 - /* we need some kind of dummy page to get a token from. This will return a response
64 - complaining that the page is missing, but we should also get an edit token */
65 - 'titles': 'DummyPageForEditToken'
66 - };
67 -
68 - var ok = function( data ) {
69 - var token;
70 - $.each( data.query.pages, function( i, page ) {
71 - if ( page['edittoken'] ) {
72 - token = page['edittoken'];
73 - return false;
 63+ var parameters = {
 64+ prop: 'info',
 65+ intoken: 'edit',
 66+ // we need some kind of dummy page to get a token from. This will return a response
 67+ // complaining that the page is missing, but we should also get an edit token
 68+ titles: 'DummyPageForEditToken'
 69+ },
 70+ ok = function( data ) {
 71+ var token;
 72+ $.each( data.query.pages, function( i, page ) {
 73+ if ( page.edittoken ) {
 74+ token = page.edittoken;
 75+ return false;
 76+ }
 77+ } );
 78+ if ( token !== undefined ) {
 79+ cachedToken = token;
 80+ tokenCallback( token );
 81+ } else {
 82+ err( 'token-missing', data );
7483 }
75 - } );
76 - if ( token !== undefined ) {
77 - cachedToken = token;
78 - tokenCallback( token );
79 - } else {
80 - err( 'token-missing', data );
81 - }
82 - };
 84+ },
 85+ ajaxOptions = {
 86+ ok: ok,
 87+ err: err,
 88+ // Due to the API assuming we're logged out if we pass the callback-parameter,
 89+ // we have to disable jQuery's callback system, and instead parse JSON string,
 90+ // by setting 'jsonp' to false.
 91+ jsonp: false
 92+ };
8393
84 - var ajaxOptions = {
85 - 'ok': ok,
86 - 'err': err,
87 - // Due to the API assuming we're logged out if we pass the callback-parameter,
88 - // we have to disable jQuery's callback system, and instead parse JSON string,
89 - // by setting 'jsonp' to false.
90 - 'jsonp': false
91 - };
92 -
93 - api.get( parameters, ajaxOptions );
 94+ return this.get( parameters, ajaxOptions );
9495 },
9596
9697 /**
9798 * Create a new section of the page.
98 - * @param {mw.Title|String} target page
99 - * @param {String} header
100 - * @param {String} wikitext message
101 - * @param {Function} success handler
102 - * @param {Function} error handler
 99+ * @param title {mw.Title|String} target page
 100+ * @param header {String}
 101+ * @param message {String} wikitext message
 102+ * @param ok {Function} success handler
 103+ * @param err {Function} error handler
 104+ * @return {jqXHR}
103105 */
104106 newSection: function( title, header, message, ok, err ) {
105107 var params = {
——@@ -109,9 +111,9 @@
110112 summary: header,
111113 text: message
112114 };
113 - this.postWithEditToken( params, ok, err );
 115+ return this.postWithEditToken( params, ok, err );
114116 }
115117
116 - } ); // end extend
 118+ } );
117119
118 -} )( window.mediaWiki, jQuery );
 120+} )( jQuery, mediaWiki );

Sign-offs

UserFlagDate
Nikerabbitinspected08:22, 22 December 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r107234Followup r107011 per Krinkle - "mediawiki.api needs to declare the mediawiki....reedy23:09, 24 December 2011
r107652[UploadWizard] Fix breakage caused by r107011...krinkle20:15, 30 December 2011
r107778[mediawiki.api] Fix typo that was made consistently....krinkle16:54, 1 January 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r105646add JS api, feedback libs from UW into core.neilk04:48, 9 December 2011
r106003change args for feedback and api -- all optional, in array.neilk10:26, 13 December 2011

Comments

#Comment by Eloquence (talk | contribs)   02:35, 24 December 2011

Quick note: Extension:UploadWizard fails to initialize as of this rev, with the following JavaScript errors (Chrome 16): 

mediawiki.api.js:22Uncaught TypeError: Cannot call method 'wikiScript' of undefined
mediawiki.api.category.js:7Uncaught TypeError: Cannot read property 'prototype' of undefined
mediawiki.api.edit.js:10Uncaught TypeError: Cannot read property 'prototype' of undefined
mediawiki.api.parse.js:7Uncaught TypeError: Cannot read property 'prototype' of undefined
mediawiki.api.titleblacklist.js:7Uncaught TypeError: Cannot read property 'prototype' of undefined
mw.UploadWizard.js:10Uncaught TypeError: undefined is not a function
#Comment by Nikerabbit (talk | contribs)   14:36, 24 December 2011

Very big commit to review :/

#Comment by Krinkle (talk | contribs)   23:05, 24 December 2011

Fortunately I know the problem. mediawiki.api needs to declare the mediawiki.util dependency.

#Comment by Eloquence (talk | contribs)   01:33, 26 December 2011

The initialization issue is indeed fixed as of r107011. However, Upload Wizard is still broken in trunk (uploads fail with "Unknown error: unknown"), which may or may not be related to this rev.

#Comment by RobLa-WMF (talk | contribs)   18:24, 29 December 2011

This is a really large change to commit, and seems likely to delay 1.19 for a week or more. Timo, is this work really worth that sort of delay?

#Comment by Nikerabbit (talk | contribs)   18:56, 29 December 2011

A week? I think I can review this in less than an hour.

#Comment by RobLa-WMF (talk | contribs)   19:25, 29 December 2011

Admittedly, I was misinterpreting a heads-up I'd gotten about a different commit. Still, isn't there some API changes that potentially ripple into other people's code here?

#Comment by Krinkle (talk | contribs)   23:10, 29 December 2011

Well, the mediawiki.api is entirely new added in trunk for 1.19, so not including this revision seems odd.

#Comment by Nikerabbit (talk | contribs)   11:11, 1 January 2012

Did you intend to call defaultsOptions as defaultOptions (the version you had in one comment)?

#Comment by Krinkle (talk | contribs)   16:54, 1 January 2012

It was named that way everywhere so no breakage, but renamed in r107778. Thanks

#Comment by He7d3r (talk | contribs)   15:00, 26 February 2012

Isn't slightly better to change part of normalizeAjaxOptions to 

var opt = typeof arg === 'function' ? { 'ok': arg }: arg;

or to 

var opt;
if ( typeof arg === 'function' ) {
	opt = { 'ok': arg };
} else {
	opt = arg;
}

Status & tagging log