r93351 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r93350‎ | r93351 | r93352 >
Date:00:43, 28 July 2011
Author:krinkle
Status:deferred (Comments)
Tags:
Comment:
AjaxCategories rewrite:

Solving syntax problems, performance improvements and applying code conventions:

* Replaced sprite image with separate images and letting ResourceLoader embed them with @embed (@embed means 0 http requests, less maintenance, none of the known limitations with sprites, and more readable code (named files rather than pixel offsets)

* Many functions were floating in the global namespace (like window.makeCaseInsensitive). A statement ends after a semi-colon(;). All functions declared after "catUrl" were assigned to the window object. I've instead turned the semi-colons back into comma's, merged some other var statements and moved them to the top of the closure. Changed local function declarations into function expressions for clarity.

* fetchSuggestions is called by $.fn.suggestions like ".call( $textbox, $textbox.val() )". So the context (this) isn't the raw element but the jQuery object, no need to re-construct with "$(this)" or "$(that)" which is slow and shouldn't even work. jQuery methods can be called on it directly. I've also replaced "$(this).val()" with the value-argument passed to fetchSuggestions which has this exact value already.

* Adding more function documentation. And changing @since to 1.19 as this was merged from js2-branch into 1.19-trunk and new features aren't backported to 1.18.

* Optimizing options/default construction to just "options = $.extend( {}, options )". Caching defaultOptions is cool, but doesn't really work if it's in a context/instance local variable. Moved it up to the module closure var statements, now it's static across all instances.

* In makeSuggestionBox(): Fixing invalid html fragments passed to jQuery that fail in IE. Shortcuts (like '<foo>' and '<foo/>') are only allowed for createElement triggers, not when creating longer fragments with content and/or attributes which are created through innerHTML, in the latter case the HTML must be completely valid and is not auto-corrected by IE.

* Using more jQuery chaining where possible.

* In buildRegex(): Using $.map with join( '|' ), (rather than $.each with += '|'; and substr).

* Storing the init instance of mw.ajaxCategories in mw.page for reference (rather than local/anonymous).

* Applied some best practices and "write testable code"
** Moved some of the functions created on the fly and assigned to 'this' into prototype (reference is cheaper)
** Making sure at least all 'do', 'set' and/or 'prototype' functions have a return value. Even if it's just a simple boolean true or context/this for chain-ability.
** Rewrote confirmEdit( .., .., .., ) as a prototyped method named "doConfirmEdit" which takes a single props-object with named valuas as argument, instead of list with 8 arguments.

* Removed trailing whitespace and other minor fixes to comply with the code conventions.
** Removed space between function name and caller: "foo ()" => foo())
** Changing "someArray.indexOf() + 1" into "someArr.indexOf() !== -1". We want a Boolean here, not a Number.
** Renamed all underscore-variables to non-underscore variants.

== Bug fixes ==

* When adding a category that is not already on the page as-is but of which the clean() version is already on the page, the script would fail. Fixed it by moving the checks up in handleCategoryAdd() and making sure that createCatLink() actually returned something.

* confirmEdit() wasn't working properly and had unused code (such as submitButton), removed hidden prepending to #catlinks, no need to, it can be dialog'ed directly from the jQuery object without being somewhere in the document.

* in doConfirmEdit() in submitFunction() and multiEdit: Clearing the input field after adding a category, so that when another category is being added it doesn't start with the previous value which is not allowed to be added again...
Modified paths:
  • /trunk/phase3/resources/mediawiki.page/images/AJAXCategorySprite.png (deleted) (history)
  • /trunk/phase3/resources/mediawiki.page/images/ajaxcat-add-hover.png (added) (history)
  • /trunk/phase3/resources/mediawiki.page/images/ajaxcat-add.png (added) (history)
  • /trunk/phase3/resources/mediawiki.page/images/ajaxcat-close-hover.png (added) (history)
  • /trunk/phase3/resources/mediawiki.page/images/ajaxcat-close.png (added) (history)
  • /trunk/phase3/resources/mediawiki.page/images/ajaxcat-edit-hover.png (added) (history)
  • /trunk/phase3/resources/mediawiki.page/images/ajaxcat-edit.png (added) (history)
  • /trunk/phase3/resources/mediawiki.page/images/ajaxcat-error-hover.png (added) (history)
  • /trunk/phase3/resources/mediawiki.page/images/ajaxcat-error.png (added) (history)
  • /trunk/phase3/resources/mediawiki.page/images/ajaxcat-tick-hover.png (added) (history)
  • /trunk/phase3/resources/mediawiki.page/images/ajaxcat-tick.png (added) (history)
  • /trunk/phase3/resources/mediawiki.page/mediawiki.page.ajaxCategories.css (modified) (history)
  • /trunk/phase3/resources/mediawiki.page/mediawiki.page.ajaxCategories.init.js (modified) (history)
  • /trunk/phase3/resources/mediawiki.page/mediawiki.page.ajaxCategories.js (modified) (history)
  • /trunk/phase3/resources/mediawiki.page/mediawiki.page.startup.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki.page/mediawiki.page.startup.js
@@ -1,5 +1,7 @@
22 ( function( $ ) {
33
 4+ mw.page = {};
 5+
46 /* Client profile classes for <html> */
57
68 var prof = $.client.profile();
Index: trunk/phase3/resources/mediawiki.page/mediawiki.page.ajaxCategories.css
@@ -9,30 +9,34 @@
1010
1111 .mw-remove-category {
1212 padding: 2px 8px;
13 - display:inline;
 13+ display: inline;
1414 }
1515 .mw-removed-category {
1616 text-decoration: line-through;
1717 }
 18+
1819 #catlinks:hover .icon {
1920 opacity: 1;
2021 }
2122 #catlinks ul {
2223 margin-right: 2em;
2324 }
 25+
2426 .mw-ajax-addcategory-holder {
2527 display: inline-block;
2628 }
2729 .mw-ajax-addcategory {
2830 margin-right: 1em;
2931 cursor: pointer;
30 - display:inline-block;
 32+ display: inline-block;
3133 }
 34+
3235 #catlinks .icon {
3336 cursor: pointer;
3437 padding: 1px 8px;
3538 margin: 0;
36 - background: url('images/AJAXCategorySprite.png') 0 0 no-repeat;
 39+ background-position: 0 0 ;
 40+ background-repeat: no-repeat;
3741 opacity: 0.5;
3842
3943 }
@@ -40,32 +44,27 @@
4145 cursor: pointer;
4246 margin-right: 1em;
4347 }
44 -#catlinks .icon-parent:hover .icon {
45 - background-position-y: -16px;
46 -}
47 -#catlinks .no-text {
48 -}
4948 #catlinks .icon-close {
50 - background-position: 0 0;
 49+ /* @embed */ background-image: url(images/ajaxcat-close.png);
5150 }
5251 #catlinks .icon-edit {
53 - background-position: -16px 0;
 52+ /* @embed */ background-image: url(images/ajaxcat-edit.png);
5453 }
5554 #catlinks .icon-tick {
56 - background-position: -32px 0;
 55+ /* @embed */ background-image: url(images/ajaxcat-tick.png);
5756 }
5857 #catlinks .icon-add {
59 - background-position: -64px 0;
 58+ /* @embed */ background-image: url(images/ajaxcat-add.png);
6059 }
6160 #catlinks .icon-close:hover {
62 - background-position: 0 -16px;
 61+ /* @embed */ background-image: url(images/ajaxcat-close-hover.png);
6362 }
6463 #catlinks .icon-edit:hover {
65 - background-position: -16px -16px;
 64+ /* @embed */ background-image: url(images/ajaxcat-edit-hover.png);
6665 }
6766 #catlinks .icon-tick:hover {
68 - background-position: -32px -16px;
 67+ /* @embed */ background-image: url(images/ajaxcat-tick-hover.png);
6968 }
7069 #catlinks .icon-add:hover {
71 - background-position: -64px -16px;
 70+ /* @embed */ background-image: url(images/ajaxcat-add-hover.png);
7271 }
Index: trunk/phase3/resources/mediawiki.page/mediawiki.page.ajaxCategories.js
@@ -3,583 +3,578 @@
44 *
55 * @author Michael Dale, 2009
66 * @author Leo Koppelkamm, 2011
7 - * @since 1.18
 7+ * @author Timo Tijhof, 2011
 8+ * @since 1.19
89 *
9 - * Relies on: mw.config (wgFormattedNamespaces, wgNamespaceIds, wgCaseSensitiveNamespaces, wgUserGroups),
10 - * mw.util.wikiGetlink, mw.user.getId
 10+ * Relies on: mw.config (wgFormattedNamespaces, wgNamespaceIds,
 11+ * wgCaseSensitiveNamespaces, wgUserGroups), mw.util.wikiGetlink, mw.user.getId
1112 */
1213 ( function( $ ) {
1314
1415 /* Local scope */
1516
1617 var catNsId = mw.config.get( 'wgNamespaceIds' ).category,
 18+ defaultOptions = {
 19+ catLinkWrapper: '<li>',
 20+ $container: $( '.catlinks' ),
 21+ $containerNormal: $( '#mw-normal-catlinks' ),
 22+ categoryLinkSelector: 'li a:not(.icon)',
 23+ multiEdit: $.inArray( 'user', mw.config.get( 'wgUserGroups' ) ) !== -1,
 24+ resolveRedirects: true
 25+ };
1726
18 - clean = function( s ) {
 27+ function clean( s ) {
1928 if ( s !== undefined ) {
2029 return s.replace( /[\x00-\x1f\x23\x3c\x3e\x5b\x5d\x7b\x7c\x7d\x7f\s]+/g, '' );
2130 }
22 - },
 31+ }
2332
2433 /**
2534 * Build URL for passed Category
26 - *
27 - * @param string category name.
28 - * @return string Valid URL
 35+ *
 36+ * @param cat {String} Category name.
 37+ * @return {String} Valid URL
2938 */
30 - catUrl = function( cat ) {
 39+ function catUrl( cat ) {
3140 return mw.util.wikiGetlink( new mw.Title( cat, catNsId ) );
32 - };
 41+ }
3342
3443 /**
35 - * Helper function for $.fn.suggestion
36 - *
37 - * @param string Query string.
 44+ * Helper function for $.fn.suggestions
 45+ *
 46+ * @context {jQuery}
 47+ * @param value {String} Textbox value.
3848 */
39 - fetchSuggestions = function( query ) {
40 - var _this = this;
41 - // ignore bad characters, they will be stripped out
42 - var catName = clean( $( this ).val() );
43 - var request = $.ajax( {
 49+ function fetchSuggestions( value ) {
 50+ var request,
 51+ $el = this,
 52+ catName = clean( value );
 53+
 54+ request = $.ajax( {
4455 url: mw.util.wikiScript( 'api' ),
4556 data: {
46 - 'action': 'query',
47 - 'list': 'allpages',
48 - 'apnamespace': catNsId,
49 - 'apprefix': catName,
50 - 'format': 'json'
 57+ action: 'query',
 58+ list: 'allpages',
 59+ apnamespace: catNsId,
 60+ apprefix: catName,
 61+ format: 'json'
5162 },
5263 dataType: 'json',
5364 success: function( data ) {
5465 // Process data.query.allpages into an array of titles
55 - var pages = data.query.allpages;
56 - var titleArr = [];
 66+ var title,
 67+ pages = data.query.allpages,
 68+ titleArr = [];
5769
5870 $.each( pages, function( i, page ) {
59 - var title = page.title.split( ':', 2 )[1];
 71+ title = page.title.split( ':', 2 )[1];
6072 titleArr.push( title );
6173 } );
6274
63 - $( _this ).suggestions( 'suggestions', titleArr );
 75+ $el.suggestions( 'suggestions', titleArr );
6476 }
6577 } );
66 - _request = request;
67 - };
68 -
 78+ $el.data( 'suggestions-request', request );
 79+ }
 80+
6981 /**
70 - * Replace <nowiki> and comments with unique keys
71 - */
72 - replaceNowikis = function( text, id, array ) {
 82+ * Replace <nowiki> and comments with unique keys
 83+ *
 84+ * @param text {String}
 85+ * @param id
 86+ * @param keys {Array}
 87+ * @return {String}
 88+ */
 89+ function replaceNowikis( text, id, keys ) {
7390 var matches = text.match( /(<nowiki\>[\s\S]*?<\/nowiki>|<\!--[\s\S]*?--\>)/g );
7491 for ( var i = 0; matches && i < matches.length; i++ ) {
75 - array[i] = matches[i];
 92+ keys[i] = matches[i];
7693 text = text.replace( matches[i], id + i + '-' );
7794 }
7895 return text;
79 - };
80 -
 96+ }
 97+
8198 /**
82 - * Restore <nowiki> and comments from unique keys
83 - */
84 - restoreNowikis = function( text, id, array ) {
85 - for ( var i = 0; i < array.length; i++ ) {
86 - text = text.replace( id + i + '-', array[i] );
 99+ * Restore <nowiki> and comments from unique keys
 100+ * @param text {String}
 101+ * @param id
 102+ * @param keys {Array}
 103+ * @return {String}
 104+ */
 105+ function restoreNowikis( text, id, keys ) {
 106+ for ( var i = 0; i < keys.length; i++ ) {
 107+ text = text.replace( id + i + '-', keys[i] );
87108 }
88109 return text;
89 - };
90 -
 110+ }
 111+
91112 /**
92113 * Makes regex string caseinsensitive.
93114 * Useful when 'i' flag can't be used.
94115 * Return stuff like [Ff][Oo][Oo]
95 - * @param string Regex string.
96 - * @return string Processed regex string
 116+ *
 117+ * @param string {String} Regex string
 118+ * @return {String} Processed regex string
97119 */
98 - makeCaseInsensitive = function( string ) {
99 - if ( $.inArray( 14, mw.config.get( 'wgCaseSensitiveNamespaces' ) ) + 1 ) {
 120+ function makeCaseInsensitive( string ) {
 121+ var newString = '';
 122+ if ( $.inArray( 14, mw.config.get( 'wgCaseSensitiveNamespaces' ) ) !== -1 ) {
100123 return string;
101124 }
102 - var newString = '';
103 - for ( var i=0; i < string.length; i++ ) {
 125+ for ( var i = 0; i < string.length; i++ ) {
104126 newString += '[' + string.charAt( i ).toUpperCase() + string.charAt( i ).toLowerCase() + ']';
105127 }
106128 return newString;
107 - };
108 -
 129+ }
 130+
109131 /**
110 - * Build a regex that matches legal invocations
111 - * of the passed category.
112 - * @param string category.
113 - * @param boolean Match one following linebreak as well?
114 - * @return Regex
 132+ * Build a regex that matches legal invocations of the passed category.
 133+ * @param category {String}
 134+ * @param matchLineBreak {Boolean} Match one following linebreak as well?
 135+ * @return {RegExp}
115136 */
116 - buildRegex = function( category, matchLineBreak ) {
117 - var categoryNSFragment = '';
118 - $.each( mw.config.get( 'wgNamespaceIds' ), function( name, id ) {
119 - if ( id == 14 ) {
120 - // The parser accepts stuff like cATegORy,
121 - // we need to do the same
122 - // ( Well unless we have wgCaseSensitiveNamespaces, but that's being checked for )
123 - categoryNSFragment += '|' + makeCaseInsensitive ( $.escapeRE( name ) );
 137+ function buildRegex( category, matchLineBreak ) {
 138+ var categoryRegex, categoryNSFragment,
 139+ titleFragment = $.escapeRE( category ).replace( /( |_)/g, '[ _]' ),
 140+ firstChar = titleFragment.charAt( 0 );
 141+
 142+ // Filter out all names for category namespace
 143+ categoryNSFragment = $.map( mw.config.get( 'wgNamespaceIds' ), function( id, name ) {
 144+ if ( id === catNsId ) {
 145+ return makeCaseInsensitive( $.escapeRE( name ) );
124146 }
125 - } );
126 - categoryNSFragment = categoryNSFragment.substr( 1 ); // Remove leading pipe
 147+ } ).join( '|' );
127148
128 - // Build the regex
129 - var titleFragment = $.escapeRE( category ).replace( /( |_)/g, '[ _]' );
130 -
131 - firstChar = titleFragment.charAt( 0 );
132149 firstChar = '[' + firstChar.toUpperCase() + firstChar.toLowerCase() + ']';
133150 titleFragment = firstChar + titleFragment.substr( 1 );
134 - var categoryRegex = '\\[\\[(' + categoryNSFragment + '):' + '[ _]*' +titleFragment + '(\\|[^\\]]*)?\\]\\]';
 151+ categoryRegex = '\\[\\[(' + categoryNSFragment + '):' + '[ _]*' + titleFragment + '(\\|[^\\]]*)?\\]\\]';
135152 if ( matchLineBreak ) {
136153 categoryRegex += '[ \\t\\r]*\\n?';
137154 }
138155 return new RegExp( categoryRegex, 'g' );
139 - };
140 -
 156+ }
141157
142 -mw.ajaxCategories = function( options ) {
143 - //Save scope in shortcut
144 - var that = this, _request, _saveAllButton, _cancelAllButton, _addContainer, defaults;
145 -
146 - defaults = {
147 - catLinkWrapper : '<li/>',
148 - $container : $( '.catlinks' ),
149 - $containerNormal : $( '#mw-normal-catlinks' ),
150 - categoryLinkSelector : 'li a:not(.icon)',
151 - multiEdit : $.inArray( 'user', mw.config.get( 'wgUserGroups' ) ) + 1,
152 - resolveRedirects : true
153 - };
154 - // merge defaults and options, without modifying defaults */
155 - options = $.extend( {}, defaults, options );
156 -
157158 /**
158 - * Insert a newly added category into the DOM
159 - *
160 - * @param string category name.
161 - * @return jQuery object
 159+ * Manufacture iconed button, with or without text.
 160+ *
 161+ * @param icon {String} The icon class.
 162+ * @param title {String} Title attribute.
 163+ * @param className {String} (optional) Additional classes to be added to the button.
 164+ * @param text {String} (optional) Text of button.
 165+ * @return {jQuery} The button.
162166 */
163 - this.createCatLink = function( cat ) {
164 - // User can implicitely state a sort key.
165 - // Remove before display
166 - cat = cat.replace(/\|.*/, '' );
 167+ function createButton( icon, title, className, text ){
 168+ // We're adding a zero width space for IE7, it's got problems with empty nodes apparently
 169+ var $button = $( '<a>' )
 170+ .addClass( className || '' )
 171+ .attr( 'title', title )
 172+ .html( '&#8203;' );
167173
168 - // strip out bad characters
169 - cat = clean ( cat );
170 -
171 - if ( $.isEmpty( cat ) || that.containsCat( cat ) ) {
172 - return;
 174+ if ( text ) {
 175+ var $icon = $( '<span>' ).addClass( 'icon ' + icon ).html( '&#8203;' );
 176+ $button.addClass( 'icon-parent' ).append( $icon ).append( text );
 177+ } else {
 178+ $button.addClass( 'icon ' + icon );
173179 }
 180+ return $button;
 181+ }
174182
175 - var $catLinkWrapper = $( options.catLinkWrapper );
176 - var $anchor = $( '<a/>' ).append( cat );
177 - $catLinkWrapper.append( $anchor );
178 - $anchor.attr( { target: "_blank", href: catUrl( cat ) } );
 183+/**
 184+ * @constructor
 185+ * @param
 186+ */
 187+mw.ajaxCategories = function( options ) {
179188
180 - _createCatButtons( $anchor );
 189+ this.options = options = $.extend( defaultOptions, options );
181190
182 - return $anchor;
183 - };
 191+ // Save scope in shortcut
 192+ var that = this;
184193
185 - /**
186 - * Takes a category link element
187 - * and strips all data from it.
188 - *
189 - * @param jQuery object
190 - */
191 - this.resetCatLink = function( $link, del, dontRestoreText ) {
192 - $link.removeClass( 'mw-removed-category mw-added-category mw-changed-category' );
193 - var data = $link.data();
 194+ // Elements tied to this instance
 195+ this.saveAllButton = null;
 196+ this.cancelAllButton = null;
 197+ this.addContainer = null;
194198
195 - if ( typeof data.stashIndex == "number" ) {
196 - _removeStashItem( data.stashIndex );
197 - }
198 - if ( del ) {
199 - $link.parent.remove();
200 - return;
201 - }
202 - if ( data.origCat && !dontRestoreText ) {
203 - $link.text( data.origCat );
204 - $link.attr( 'href', catUrl( data.origCat ) );
205 - }
 199+ this.request = null;
206200
207 - $link.removeData();
208 -
209 - //Readd static.
210 - $link.data({
211 - saveButton : data.saveButton,
212 - deleteButton: data.deleteButton,
213 - editButton : data.editButton
214 - });
 201+ // Stash and hooks
 202+ this.stash = {
 203+ summaries: [],
 204+ shortSum: [],
 205+ fns: []
215206 };
 207+ this.hooks = {
 208+ beforeAdd: [],
 209+ beforeChange: [],
 210+ beforeDelete: [],
 211+ afterAdd: [],
 212+ afterChange: [],
 213+ afterDelete: []
 214+ };
216215
 216+ /* Event handlers */
 217+
217218 /**
218 - * Reset all data from the category links and the stash.
219 - * @param Boolean del Delete any category links with .mw-removed-category
 219+ * Handle add category submit. Not to be called directly.
 220+ *
 221+ * @context Element
 222+ * @param e {jQuery Event}
220223 */
221 - this.resetAll = function( del ) {
222 - var $links = options.$container.find( options.categoryLinkSelector ), $del = $();
223 - if ( del ) {
224 - $del = $links.filter( '.mw-removed-category' ).parent();
225 - }
 224+ this.handleAddLink = function( e ) {
 225+ var $el = $( this ),
 226+ $link = $([]),
 227+ categoryText = $.ucFirst( $el.parent().find( '.mw-addcategory-input' ).val() || '' );
226228
227 - $links.each( function() {
228 - that.resetCatLink( $( this ), false, del );
229 - });
230 -
231 - $del.remove();
 229+ // Resolve redirects
 230+ that.resolveRedirects( categoryText, function( resolvedCat, exists ) {
 231+ that.handleCategoryAdd( $link, resolvedCat, false, exists );
 232+ } );
 233+ };
232234
233 - if ( !options.$container.find( '#mw-hidden-catlinks li' ).length ) {
234 - options.$container.find( '#mw-hidden-catlinks' ).remove();
235 - }
236 - };
237 -
238235 /**
239 - * Create a suggestion box for use in edit/add dialogs
240 - * @param str prefill Prefill input
241 - * @param function callback on submit
242 - * @param str buttonVal Button text
 236+ * @context Element
 237+ * @param e {jQuery Event}
243238 */
244 - this._makeSuggestionBox = function( prefill, callback, buttonVal ) {
245 - // Create add category prompt
246 - var promptContainer = $( '<div class="mw-addcategory-prompt"/>' );
247 - var promptTextbox = $( '<input type="text" size="30" class="mw-addcategory-input"/>' );
248 - if ( prefill !== '' ) {
249 - promptTextbox.val( prefill );
250 - }
251 - var addButton = $( '<input type="button" class="mw-addcategory-button"/>' );
252 - addButton.val( buttonVal );
 239+ this.createEditInterface = function( e ) {
 240+ var $el = $( this ),
 241+ $link = $el.data( 'link' ),
 242+ category = $link.text(),
 243+ $input = that.makeSuggestionBox( category,
 244+ that.handleEditLink,
 245+ that.options.multiEdit ? mw.msg( 'ajax-confirm-ok' ) : mw.msg( 'ajax-confirm-save' )
 246+ );
253247
254 - addButton.click( callback );
255 - promptTextbox.keyup( function( e ) {
256 - if ( e.keyCode == 13 ) addButton.click();
257 - });
258 - promptTextbox.suggestions( {
259 - 'fetch': fetchSuggestions,
260 - 'cancel': function() {
261 - var req = _request;
262 - // XMLHttpRequest.abort is unimplemented in IE6, also returns nonstandard value of "unknown" for typeof
263 - if ( req && ( typeof req.abort !== 'unknown' ) && ( typeof req.abort !== 'undefined' ) && req.abort ) {
264 - req.abort();
265 - }
266 - }
267 - } );
 248+ $link.after( $input ).hide();
268249
269 - promptTextbox.suggestions();
 250+ $input.find( '.mw-addcategory-input' ).focus();
270251
271 - promptContainer.append( promptTextbox );
272 - promptContainer.append( addButton );
 252+ $link.data( 'editButton' ).hide();
273253
274 - return promptContainer;
 254+ $link.data( 'deleteButton' )
 255+ .unbind( 'click' )
 256+ .click( function() {
 257+ $input.remove();
 258+ $link.show().data( 'editButton' ).show();
 259+ $( this )
 260+ .unbind( 'click' )
 261+ .click( that.handleDeleteLink )
 262+ .attr( 'title', mw.msg( 'ajax-remove-category' ) );
 263+ })
 264+ .attr( 'title', mw.msg( 'ajax-cancel' ) );
275265 };
276266
277267 /**
278 - * Parse the DOM $container and build a list of
279 - * present categories
280 - *
281 - * @return array Array of all categories
 268+ * Handle edit category submit. Not to be called directly.
 269+ *
 270+ * @context Element
 271+ * @param e {jQuery Event}
282272 */
283 - this.getCats = function() {
284 - return options.$container.find( options.categoryLinkSelector ).map( function() { return $.trim( $( this ).text() ); } );
285 - };
 273+ this.handleEditLink = function( e ) {
 274+ var categoryNew,
 275+ $el = $( this ),
 276+ $link = $el.parent().parent().find( 'a:not(.icon)' ),
 277+ sortkey = '';
286278
287 - /**
288 - * Check whether a passed category is present in the DOM
289 - *
290 - * @return boolean True for exists
291 - */
292 - this.containsCat = function( cat ) {
293 - return that.getCats().filter( function() { return $.ucFirst( this ) == $.ucFirst( cat ); } ).length !== 0;
 279+ // Grab category text
 280+ categoryNew = $el.parent().find( '.mw-addcategory-input' ).val();
 281+ categoryNew = $.ucFirst( categoryNew.replace( /_/g, ' ' ) );
 282+
 283+ // Strip sortkey
 284+ var arr = categoryNew.split( '|' );
 285+ if ( arr.length > 1 ) {
 286+ categoryNew = arr.shift();
 287+ sortkey = '|' + arr.join( '|' );
 288+ }
 289+
 290+ // Grab text
 291+ var added = $link.hasClass( 'mw-added-category' );
 292+ that.resetCatLink( $link );
 293+ var category = $link.text();
 294+
 295+ // Check for dupes ( exluding itself )
 296+ if ( category !== categoryNew && that.containsCat( categoryNew ) ) {
 297+ $link.data( 'deleteButton' ).click();
 298+ return;
 299+ }
 300+
 301+ // Resolve redirects
 302+ that.resolveRedirects( categoryNew, function( resolvedCat, exists ) {
 303+ that.handleCategoryEdit( $link, category, resolvedCat, sortkey, exists, added );
 304+ });
294305 };
295306
296307 /**
297 - * This gets called by all action buttons
298 - * Displays a dialog to confirm the action
299 - * Afterwards do the actual edit
 308+ * Handle delete category submit. Not to be called directly.
300309 *
301 - * @param function fn text-modifying function
302 - * @param string actionSummary Changes done
303 - * @param string shortSummary Changes, short version
304 - * @param function fn doneFn callback after everything is done
305 - * @return boolean True for exists
 310+ * @context Element
 311+ * @param e {jQuery Event}
306312 */
307 - this._confirmEdit = function( fn, actionSummary, shortSummary, doneFn, $link, action ) {
308 - // Check whether to use multiEdit mode
309 - if ( options.multiEdit && action != 'all' ) {
310 - // Stash away
311 - $link.data( 'stashIndex', _stash.fns.length );
312 - $link.data( 'summary', actionSummary );
313 - _stash.summaries.push( actionSummary );
314 - _stash.shortSum.push( shortSummary );
315 - _stash.fns.push( fn );
 313+ this.handleDeleteLink = function( e ) {
 314+ var $el = $( this ),
 315+ $link = $el.parent().find( 'a:not(.icon)' ),
 316+ category = $link.text();
316317
317 - _saveAllButton.show();
318 - _cancelAllButton.show();
319 -
320 - // This only does visual changes
321 - doneFn( true );
 318+ if ( $link.is( '.mw-added-category, .mw-changed-category' ) ) {
 319+ // We're just cancelling the addition or edit
 320+ that.resetCatLink( $link, $link.hasClass( 'mw-added-category' ) );
322321 return;
 322+ } else if ( $link.is( '.mw-removed-category' ) ) {
 323+ // It's already removed...
 324+ return;
323325 }
324 - // Produce a confirmation dialog
325 - var dialog = $( '<div/>' );
326 -
327 - dialog.addClass( 'mw-ajax-confirm-dialog' );
328 - dialog.attr( 'title', mw.msg( 'ajax-confirm-title' ) );
329 -
330 - // Summary of the action to be taken
331 - var summaryHolder = $( '<p/>' );
332 - summaryHolder.html( '<strong>' + mw.msg( 'ajax-category-question' ) + '</strong><br>' + actionSummary );
333 - dialog.append( summaryHolder );
334 -
335 - // Reason textbox.
336 - var reasonBox = $( '<input type="text" size="45" />' );
337 - reasonBox.addClass( 'mw-ajax-confirm-reason' );
338 - dialog.append( reasonBox );
339 -
340 - // Submit button
341 - var submitButton = $( '<input type="button"/>' );
342 - submitButton.val( mw.msg( 'ajax-confirm-save' ) );
343 -
344 - var submitFunction = function() {
345 - that._addProgressIndicator( dialog );
346 - that._doEdit(
347 - mw.config.get( 'wgPageName' ),
348 - fn,
349 - shortSummary + ': ' + reasonBox.val(),
350 - function() {
351 - doneFn();
352 - dialog.dialog( 'close' );
353 - that._removeProgressIndicator( dialog );
354 - }
355 - );
356 - };
357 -
358 - var buttons = {};
359 - buttons[mw.msg( 'ajax-confirm-save' )] = submitFunction;
360 - var dialogOptions = {
361 - 'AutoOpen' : true,
362 - 'buttons' : buttons,
363 - 'width' : 450
364 - };
365 -
366 - $( '#catlinks' ).prepend( dialog );
367 - dialog.dialog( dialogOptions );
368 -
369 - // Close on enter
370 - dialog.keyup( function( e ) {
371 - if ( e.keyCode == 13 ) submitFunction();
372 - });
 326+ that.handleCategoryDelete( $link, category );
373327 };
374328
375329 /**
376330 * When multiEdit mode is enabled,
377331 * this is called when the user clicks "save all"
378 - * Combines the summaries and edit functions
 332+ * Combines the summaries and edit functions.
 333+ *
 334+ * @context Element
 335+ * @return ?
379336 */
380 - this._handleStashedCategories = function() {
381 - var summary = '', fns = _stash.fns;
 337+ this.handleStashedCategories = function() {
382338
383339 // Remove "holes" in array
384 - summary = $.grep( _stash.summaries, function( n, i ) {
385 - return ( n );
386 - });
 340+ var summary = $.grep( that.stash.summaries, function( n, i ) {
 341+ return n;
 342+ } );
 343+
387344 if ( summary.length < 1 ) {
388345 // Nothing to do here.
389 - _saveAllButton.hide();
390 - _cancelAllButton.hide();
 346+ that.saveAllButton.hide();
 347+ that.cancelAllButton.hide();
391348 return;
392349 } else {
393 - summary = summary.join( '<br>' );
 350+ summary = summary.join( '<br/>' );
394351 }
 352+
395353 // Remove "holes" in array
396 - summaryShort = $.grep( _stash.shortSum, function( n,i ) {
397 - return ( n );
398 - });
 354+ var summaryShort = $.grep( that.stash.shortSum, function( n,i ) {
 355+ return n;
 356+ } );
399357 summaryShort = summaryShort.join( ', ' );
400358
401 - var combinedFn = function( oldtext ) {
402 - // Run the text through all action functions
403 - newtext = oldtext;
404 - for ( var i = 0; i < fns.length; i++ ) {
405 - if ( $.isFunction( fns[i] ) ) {
406 - newtext = fns[i]( newtext );
407 - if ( newtext === false ) {
408 - return false;
 359+ var fns = that.stash.fns;
 360+
 361+ that.doConfirmEdit( {
 362+ modFn: function( oldtext ) {
 363+ // Run the text through all action functions
 364+ var newtext = oldtext;
 365+ for ( var i = 0; i < fns.length; i++ ) {
 366+ if ( $.isFunction( fns[i] ) ) {
 367+ newtext = fns[i]( newtext );
 368+ if ( newtext === false ) {
 369+ return false;
 370+ }
409371 }
410372 }
411 - }
412 - return newtext;
413 - };
414 - var doneFn = function() { that.resetAll( true ); };
415 -
416 - that._confirmEdit( combinedFn, summary, summaryShort, doneFn, '', 'all' );
 373+ return newtext;
 374+ },
 375+ actionSummary: summary,
 376+ shortSummary: summaryShort,
 377+ doneFn: function() {
 378+ that.resetAll( true );
 379+ },
 380+ $link: null,
 381+ action: 'all'
 382+ } );
417383 };
 384+};
418385
 386+/* Public methods */
 387+
 388+mw.ajaxCategories.prototype = {
419389 /**
420 - * Do the actual edit.
421 - * Gets token & text from api, runs it through fn
422 - * and saves it with summary.
423 - * @param str page Pagename
424 - * @param function fn edit function
425 - * @param str summary
426 - * @param str doneFn Callback after all is done
 390+ * Create the UI
427391 */
428 - this._doEdit = function( page, fn, summary, doneFn ) {
429 - // Get an edit token for the page.
430 - var getTokenVars = {
431 - 'action':'query',
432 - 'prop':'info|revisions',
433 - 'intoken':'edit',
434 - 'titles':page,
435 - 'rvprop':'content|timestamp',
436 - 'format':'json'
437 - };
 392+ setup: function() {
 393+ // Could be set by gadgets like HotCat etc.
 394+ if ( mw.config.get( 'disableAJAXCategories' ) ) {
 395+ return false;
 396+ }
 397+ // Only do it for articles.
 398+ if ( !mw.config.get( 'wgIsArticle' ) ) {
 399+ return;
 400+ }
 401+ var options = this.options,
 402+ that = this,
 403+ // Create [Add Category] link
 404+ $addLink = createButton( 'icon-add',
 405+ mw.msg( 'ajax-add-category' ),
 406+ 'mw-ajax-addcategory',
 407+ mw.msg( 'ajax-add-category' )
 408+ ).click( function() {
 409+ $( this ).nextAll().toggle().filter( '.mw-addcategory-input' ).focus();
 410+ });
438411
439 - $.post( mw.util.wikiScript( 'api' ), getTokenVars,
440 - function( reply ) {
441 - var infos = reply.query.pages;
442 - $.each(
443 - infos,
444 - function( pageid, data ) {
445 - var token = data.edittoken;
446 - var timestamp = data.revisions[0].timestamp;
447 - var oldText = data.revisions[0]['*'];
 412+ // Create add category prompt
 413+ this.addContainer = this.makeSuggestionBox( '', this.handleAddLink, mw.msg( 'ajax-add-category-submit' ) );
 414+ this.addContainer.children().hide();
 415+ this.addContainer.prepend( $addLink );
448416
449 - // Replace all nowiki and comments with unique keys
450 - var key = mw.user.generateId();
451 - var nowiki = [];
452 - oldText = replaceNowikis( oldText, key, nowiki );
453 -
454 - // Then do the changes
455 - var newText = fn( oldText );
456 - if ( newText === false ) return;
457 -
458 - // And restore them back
459 - newText = restoreNowikis( newText, key, nowiki );
 417+ // Create edit & delete link for each category.
 418+ $( '#catlinks li a' ).each( function() {
 419+ that.createCatButtons( $( this ) );
 420+ });
460421
461 - var postEditVars = {
462 - 'action':'edit',
463 - 'title':page,
464 - 'text':newText,
465 - 'summary':summary,
466 - 'token':token,
467 - 'basetimestamp':timestamp,
468 - 'format':'json'
469 - };
 422+ options.$containerNormal.append( this.addContainer );
470423
471 - $.post( mw.util.wikiScript( 'api' ), postEditVars, doneFn, 'json' )
472 - .error( function( xhr, text, error ) {
473 - _showError( mw.msg( 'ajax-api-error', text, error ) );
474 - });
475 - }
476 - );
477 - }
478 - , 'json' ).error( function( xhr, text, error ) {
479 - _showError( mw.msg( 'ajax-api-error', text, error ) );
480 - });
481 - };
 424+ // @todo Make more clickable
 425+ this.saveAllButton = createButton( 'icon-tick',
 426+ mw.msg( 'ajax-confirm-save-all' ),
 427+ '',
 428+ mw.msg( 'ajax-confirm-save-all' )
 429+ );
 430+ this.cancelAllButton = createButton( 'icon-close',
 431+ mw.msg( 'ajax-cancel-all' ),
 432+ '',
 433+ mw.msg( 'ajax-cancel-all' )
 434+ );
 435+ this.saveAllButton.click( this.handleStashedCategories ).hide();
 436+ this.cancelAllButton.click( function() {
 437+ that.resetAll( false );
 438+ } ).hide();
 439+ options.$containerNormal.append( this.saveAllButton ).append( this.cancelAllButton );
 440+ options.$container.append( this.addContainer );
 441+ },
 442+
482443 /**
483 - * Append spinner wheel to element
484 - * @param DOMObject element.
 444+ * Insert a newly added category into the DOM.
 445+ *
 446+ * @param cat {String} Category name.
 447+ * @return {jQuery}
485448 */
486 - this._addProgressIndicator = function( elem ) {
487 - elem.append( $( '<div/>' ).addClass( 'mw-ajax-loader' ) );
488 - };
 449+ createCatLink: function( cat ) {
 450+ // User can implicitely state a sort key.
 451+ // Remove before display.
 452+ // strip out bad characters
 453+ cat = clean( cat.replace( /\|.*/, '' ) );
489454
490 - /**
491 - * Find and remove spinner wheel from inside element
492 - * @param DOMObject parent element.
493 - */
494 - this._removeProgressIndicator = function( elem ) {
495 - elem.find( '.mw-ajax-loader' ).remove();
496 - };
497 -
498 - /**
499 - * Checks the API whether the category in question is a redirect.
500 - * Also returns existance info ( to color link red/blue )
501 - * @param string category.
502 - * @param function callback
503 - */
504 - this._resolveRedirects = function( category, callback ) {
505 - if ( !options.resolveRedirects ) {
506 - callback( category );
 455+ if ( $.isEmpty( cat ) || this.containsCat( cat ) ) {
507456 return;
508457 }
509 - var queryVars = {
510 - 'action':'query',
511 - 'titles': new mw.Title( category, catNsId ).toString(),
512 - 'redirects':'',
513 - 'format' : 'json'
514 - };
515458
516 - $.get( mw.util.wikiScript( 'api' ), queryVars,
517 - function( reply ) {
518 - var redirect = reply.query.redirects;
519 - if ( redirect ) {
520 - category = new mw.Title( redirect[0].to )._name;
521 - }
522 - callback( category, !reply.query.pages[-1] );
523 - }
524 - , 'json' );
525 - };
526 -
 459+ var $catLinkWrapper = $( this.options.catLinkWrapper ),
 460+ $anchor = $( '<a>' )
 461+ .append( cat )
 462+ .attr( {
 463+ target: '_blank',
 464+ href: catUrl( cat )
 465+ } );
 466+
 467+ $catLinkWrapper.append( $anchor );
 468+
 469+ this.createCatButtons( $anchor );
 470+
 471+ return $anchor;
 472+ },
 473+
527474 /**
528 - * Handle add category submit. Not to be called directly
 475+ * Create a suggestion box for use in edit/add dialogs
 476+ * @param prefill {String} Prefill input
 477+ * @param callback {Function} Called on submit
 478+ * @param buttonVal {String} Button text
529479 */
530 - this._handleAddLink = function( e ) {
531 - var $this = $( this ), $link = $();
 480+ makeSuggestionBox: function( prefill, callback, buttonVal ) {
 481+ // Create add category prompt
 482+ var $promptContainer = $( '<div class="mw-addcategory-prompt"></div>' ),
 483+ $promptTextbox = $( '<input type="text" size="30" class="mw-addcategory-input"></input>' ),
 484+ $addButton = $( '<input type="button" class="mw-addcategory-button"></input>' ),
 485+ that = this;
532486
533 - // Grab category text
534 - var category = $this.parent().find( '.mw-addcategory-input' ).val();
535 - category = $.ucFirst( category );
 487+ if ( prefill !== '' ) {
 488+ $promptTextbox.val( prefill );
 489+ }
536490
537 - // Resolve redirects
538 - that._resolveRedirects( category, function( resolvedCat, exists ) {
539 - that.handleCategoryAdd( $link, resolvedCat, false, exists );
540 - } );
541 - };
 491+ $addButton
 492+ .val( buttonVal )
 493+ .click( callback );
 494+
 495+ $promptTextbox
 496+ .keyup( function( e ) {
 497+ if ( e.keyCode === 13 ) {
 498+ $addButton.click();
 499+ }
 500+ } )
 501+ .suggestions( {
 502+ fetch: fetchSuggestions,
 503+ cancel: function() {
 504+ var req = this.data( 'suggestions-request' );
 505+ // XMLHttpRequest.abort is unimplemented in IE6, also returns nonstandard value of 'unknown' for typeof
 506+ if ( req && typeof req.abort !== 'unknown' && typeof req.abort !== 'undefined' && req.abort ) {
 507+ req.abort();
 508+ }
 509+ }
 510+ } )
 511+ .suggestions();
 512+
 513+ $promptContainer
 514+ .append( $promptTextbox )
 515+ .append( $addButton );
 516+
 517+ return $promptContainer;
 518+ },
 519+
542520 /**
543 - * Execute or queue an category add
 521+ * Execute or queue an category add.
 522+ * @param $link {jQuery}
 523+ * @param category
 524+ * @param noAppend
 525+ * @param exists
 526+ * @return {mw.ajaxCategories}
544527 */
545 - this.handleCategoryAdd = function( $link, category, noAppend, exists ) {
546 - if ( !$link.length ) {
547 - $link = that.createCatLink( category );
548 - }
549 - // Mark red if missing
550 - $link.toggleClass( 'new', exists === false );
551 -
 528+ handleCategoryAdd: function( $link, category, noAppend, exists ) {
552529 // Handle sortkey
553 - var arr = category.split( '|' ), sortkey = '';
554 -
 530+ var arr = category.split( '|' ),
 531+ sortkey = '',
 532+ that = this;
 533+
555534 if ( arr.length > 1 ) {
556535 category = arr.shift();
557536 sortkey = '|' + arr.join( '|' );
558 - if ( sortkey == '|' ) sortkey = '';
 537+ if ( sortkey === '|' ) {
 538+ sortkey = '';
 539+ }
559540 }
560 -
561 - //Replace underscores
562 - category = category.replace(/_/g, ' ' );
563 -
564 - if ( that.containsCat( category ) ) {
565 - _showError( mw.msg( 'ajax-category-already-present', category ) );
566 - return;
 541+
 542+ if ( !$link.length ) {
 543+ $link = this.createCatLink( category );
567544 }
568 - var catFull = new mw.Title( category, catNsId ).toString().replace(/_/g, ' ' );
569 - var appendText = "\n[[" + catFull + sortkey + "]]\n";
570 - var summary = mw.msg( 'ajax-add-category-summary', category );
571 - var shortSummary = '+[[' + catFull + ']]';
572 - that._confirmEdit(
573 - function( oldText ) {
574 - newText = _runHooks ( oldText, 'beforeAdd', category );
575 - newText = newText + appendText;
576 - return _runHooks ( newText, 'afterAdd', category );
 545+
 546+ if ( this.containsCat( category ) ) {
 547+ this.showError( mw.msg( 'ajax-category-already-present', category ) );
 548+ return this;
 549+ }
 550+
 551+ // Sometimes createCatLink returns undefined/null, previously caused an exception
 552+ // in the following lines, catching now.. @todo
 553+ if ( !$link ) {
 554+ this.showError( 'Unexpected error occurred. $link undefined.' );
 555+ return this;
 556+ }
 557+
 558+ // Mark red if missing
 559+ $link.toggleClass( 'new', exists !== true );
 560+
 561+ // Replace underscores
 562+ category = category.replace( /_/g, ' ' );
 563+ var catFull = new mw.Title( category, catNsId ).toString().replace( /_/g, ' ' );
 564+
 565+ this.doConfirmEdit( {
 566+ modFn: function( oldText ) {
 567+ var newText = that.runHooks( oldText, 'beforeAdd', category );
 568+ newText = newText + "\n[[" + catFull + sortkey + "]]\n";
 569+ return that.runHooks( newText, 'afterAdd', category );
577570 },
578 - summary,
579 - shortSummary,
580 - function( unsaved ) {
 571+ actionSummary: mw.msg( 'ajax-add-category-summary', category ),
 572+ shortSummary: '+[[' + catFull + ']]',
 573+ doneFn: function( unsaved ) {
581574 if ( !noAppend ) {
582 - options.$container.find( '#mw-normal-catlinks>.mw-addcategory-prompt' ).children( 'input' ).hide();
583 - options.$container.find( '#mw-normal-catlinks ul' ).append( $link.parent() );
 575+ that.options.$container
 576+ .find( '#mw-normal-catlinks > .mw-addcategory-prompt' ).children( 'input' ).hide();
 577+ that.options.$container
 578+ .find( '#mw-normal-catlinks ul' ).append( $link.parent() );
584579 } else {
585580 // Remove input box & button
586581 $link.data( 'deleteButton' ).click();
@@ -592,115 +587,70 @@
593588 }
594589 $( '.mw-ajax-addcategory' ).click();
595590 },
596 - $link,
597 - 'add'
598 - );
599 - };
600 - this._createEditInterface = function( e ) {
601 - var $this = $( this ),
602 - $link = $this.data( 'link' ),
603 - category = $link.text();
604 - var $input = that._makeSuggestionBox( category,
605 - that._handleEditLink,
606 - options.multiEdit ? mw.msg( 'ajax-confirm-ok' ) : mw.msg( 'ajax-confirm-save' )
607 - );
608 - $link.after( $input ).hide();
609 - $input.find( '.mw-addcategory-input' ).focus();
610 - $link.data( 'editButton' ).hide();
611 - $link.data( 'deleteButton' ).unbind( 'click' ).click( function() {
612 - $input.remove();
613 - $link.show();
614 - $link.data( 'editButton' ).show();
615 - $( this ).unbind( 'click' ).click( that._handleDeleteLink )
616 - .attr( 'title', mw.msg( 'ajax-remove-category' ));
617 - }).attr( 'title', mw.msg( 'ajax-cancel' ));
618 - };
619 -
 591+ $link: $link,
 592+ action: 'add'
 593+ } );
 594+ return this;
 595+ },
 596+
620597 /**
621 - * Handle edit category submit. Not to be called directly
 598+ * Execute or queue an category edit.
 599+ * @param $link {jQuery}
 600+ * @param category
 601+ * @param categoryNew
 602+ * @param sortkeyNew
 603+ * @param exists {Boolean}
 604+ * @param added {Boolean}
622605 */
623 - this._handleEditLink = function( e ) {
624 - var $this = $( this ),
625 - $link = $this.parent().parent().find( 'a:not(.icon)' ),
626 - categoryNew, sortkey = '';
627 -
628 - // Grab category text
629 - categoryNew = $this.parent().find( '.mw-addcategory-input' ).val();
630 - categoryNew = $.ucFirst( categoryNew.replace(/_/g, ' ' ) );
631 -
632 - // Strip sortkey
633 - var arr = categoryNew.split( '|' );
634 - if ( arr.length > 1 ) {
635 - categoryNew = arr.shift();
636 - sortkey = '|' + arr.join( '|' );
637 - }
 606+ handleCategoryEdit: function( $link, category, categoryNew, sortkeyNew, exists, added ) {
 607+ var that = this;
638608
639 - // Grab text
640 - var added = $link.hasClass( 'mw-added-category' );
641 - that.resetCatLink ( $link );
642 - var category = $link.text();
643 -
644 - // Check for dupes ( exluding itself )
645 - if ( category != categoryNew && that.containsCat( categoryNew ) ) {
646 - $link.data( 'deleteButton' ).click();
647 - return;
648 - }
649 -
650 - // Resolve redirects
651 - that._resolveRedirects( categoryNew, function( resolvedCat, exists ) {
652 - that.handleCategoryEdit( $link, category, resolvedCat, sortkey, exists, added );
653 - });
654 - };
655 - /**
656 - * Execute or queue an category edit
657 - */
658 - this.handleCategoryEdit = function( $link, category, categoryNew, sortkeyNew, exists, added ) {
659609 // Category add needs to be handled differently
660610 if ( added ) {
661611 // Pass sortkey back
662612 that.handleCategoryAdd( $link, categoryNew + sortkeyNew, true );
663613 return;
664614 }
 615+
665616 // User didn't change anything.
666 - if ( category == categoryNew + sortkeyNew ) {
 617+ if ( category === categoryNew + sortkeyNew ) {
667618 $link.data( 'deleteButton' ).click();
668619 return;
669620 }
 621+
670622 // Mark red if missing
671 - $link.toggleClass( 'new', exists === false );
672 -
673 - categoryRegex = buildRegex( category );
674 -
675 - var summary = mw.msg( 'ajax-edit-category-summary', category, categoryNew );
676 - var shortSummary = '[[' + new mw.Title( category, catNsId ) + ']] -> [[' + new mw.Title( categoryNew, catNsId ) + ']]';
677 - that._confirmEdit(
678 - function( oldText ) {
679 - newText = _runHooks ( oldText, 'beforeChange', category, categoryNew );
 623+ $link[(exists === false ? 'addClass' : 'removeClass')]( 'new' );
680624
681 - var matches = newText.match( categoryRegex );
 625+ var categoryRegex = buildRegex( category ),
 626+ shortSummary = '[[' + new mw.Title( category, catNsId ) + ']] -> [[' + new mw.Title( categoryNew, catNsId ) + ']]';
 627+ that.doConfirmEdit({
 628+ modFn: function( oldText ) {
 629+ var sortkey, newCategoryString,
 630+ newText = that.runHooks( oldText, 'beforeChange', category, categoryNew ),
 631+ matches = newText.match( categoryRegex );
682632
683633 //Old cat wasn't found, likely to be transcluded
684634 if ( !$.isArray( matches ) ) {
685 - _showError( mw.msg( 'ajax-edit-category-error' ) );
 635+ that.showError( mw.msg( 'ajax-edit-category-error' ) );
686636 return false;
687637 }
688 - var sortkey = sortkeyNew || matches[0].replace( categoryRegex, '$2' );
689 - var newCategoryString = "[[" + new mw.Title( categoryNew, catNsId ) + sortkey + ']]';
 638+ sortkey = sortkeyNew || matches[0].replace( categoryRegex, '$2' );
 639+ newCategoryString = '[[' + new mw.Title( categoryNew, catNsId ) + sortkey + ']]';
690640
691641 if ( matches.length > 1 ) {
692642 // The category is duplicated.
693643 // Remove all but one match
694644 for ( var i = 1; i < matches.length; i++ ) {
695 - oldText = oldText.replace( matches[i], '' );
 645+ oldText = oldText.replace( matches[i], '' );
696646 }
697647 }
698 - var newText = oldText.replace( categoryRegex, newCategoryString );
 648+ newText = oldText.replace( categoryRegex, newCategoryString );
699649
700 - return _runHooks ( newText, 'afterChange', category, categoryNew );
 650+ return that.runHooks( newText, 'afterChange', category, categoryNew );
701651 },
702 - summary,
703 - shortSummary,
704 - function( unsaved ) {
 652+ actionSummary: mw.msg( 'ajax-edit-category-summary', category, categoryNew ),
 653+ shortSummary: shortSummary,
 654+ doneFn: function( unsaved ) {
705655 // Remove input box & button
706656 $link.data( 'deleteButton' ).click();
707657
@@ -710,246 +660,397 @@
711661 $link.data( 'origCat', category ).addClass( 'mw-changed-category' );
712662 }
713663 },
714 - $link,
715 - 'edit'
716 - );
717 - };
718 -
 664+ $link: $link,
 665+ action: 'edit'
 666+ });
 667+ },
 668+
719669 /**
720 - * Handle delete category submit. Not to be called directly
 670+ * Checks the API whether the category in question is a redirect.
 671+ * Also returns existance info (to color link red/blue)
 672+ * @param string category.
 673+ * @param function callback
721674 */
722 - this._handleDeleteLink = function() {
723 - var $this = $( this ),
724 - $link = $this.parent().find( 'a:not(.icon)' ),
725 - category = $link.text();
726 -
727 - if ( $link.is( '.mw-added-category, .mw-changed-category' ) ) {
728 - // We're just cancelling the addition or edit
729 - that.resetCatLink ( $link, $link.hasClass( 'mw-added-category' ) );
 675+ resolveRedirects: function( category, callback ) {
 676+ if ( !this.options.resolveRedirects ) {
 677+ callback( category, true );
730678 return;
731 - } else if ( $link.is( '.mw-removed-category' ) ) {
732 - // It's already removed...
733 - return;
734679 }
735 - that.handleCategoryDelete( $link, category );
736 - };
737 -
 680+ var queryVars = {
 681+ action:'query',
 682+ titles: new mw.Title( category, catNsId ).toString(),
 683+ redirects: '',
 684+ format: 'json'
 685+ };
 686+
 687+ $.get( mw.util.wikiScript( 'api' ), queryVars,
 688+ function( reply ) {
 689+ var redirect = reply.query.redirects;
 690+ if ( redirect ) {
 691+ category = new mw.Title( redirect[0].to )._name;
 692+ }
 693+ callback( category, !reply.query.pages[-1] );
 694+ }, 'json'
 695+ );
 696+ },
 697+
738698 /**
739 - * Execute or queue an category delete
 699+ * Append edit and remove buttons to a given category link
 700+ *
 701+ * @param DOMElement element Anchor element, to which the buttons should be appended.
 702+ * @return {mw.ajaxCategories}
740703 */
741 - this.handleCategoryDelete = function( $link, category ) {
742 - var categoryRegex = buildRegex( category, true );
 704+ createCatButtons: function( $element ) {
 705+ var deleteButton = createButton( 'icon-close', mw.msg( 'ajax-remove-category' ) ),
 706+ editButton = createButton( 'icon-edit', mw.msg( 'ajax-edit-category' ) ),
 707+ saveButton = createButton( 'icon-tick', mw.msg( 'ajax-confirm-save' ) ).hide(),
 708+ that = this;
743709
744 - var summary = mw.msg( 'ajax-remove-category-summary', category );
745 - var shortSummary = '-[[' + new mw.Title( category, catNsId ) + ']]';
 710+ deleteButton.click( this.handleDeleteLink );
 711+ editButton.click( that.createEditInterface );
746712
747 - that._confirmEdit(
748 - function( oldText ) {
749 - newText = _runHooks ( oldText, 'beforeDelete', category );
750 - var newText = newText.replace( categoryRegex, '' );
 713+ $element.after( deleteButton ).after( editButton );
751714
752 - if ( newText == oldText ) {
753 - _showError( mw.msg( 'ajax-remove-category-error' ) );
 715+ // Save references to all links and buttons
 716+ $element.data( {
 717+ deleteButton: deleteButton,
 718+ editButton: editButton,
 719+ saveButton: saveButton
 720+ } );
 721+ editButton.data( {
 722+ link: $element
 723+ } );
 724+ return this;
 725+ },
 726+
 727+ /**
 728+ * Append spinner wheel to element.
 729+ * @param $el {jQuery}
 730+ * @return {mw.ajaxCategories}
 731+ */
 732+ addProgressIndicator: function( $el ) {
 733+ $el.append( $( '<div>' ).addClass( 'mw-ajax-loader' ) );
 734+ return this;
 735+ },
 736+
 737+ /**
 738+ * Find and remove spinner wheel from inside element.
 739+ * @param $el {jQuery}
 740+ * @return {mw.ajaxCategories}
 741+ */
 742+ removeProgressIndicator: function( $el ) {
 743+ $el.find( '.mw-ajax-loader' ).remove();
 744+ return this;
 745+ },
 746+
 747+ /**
 748+ * Parse the DOM $container and build a list of
 749+ * present categories.
 750+ *
 751+ * @return {Array} All categories.
 752+ */
 753+ getCats: function() {
 754+ var cats = this.options.$container
 755+ .find( this.options.categoryLinkSelector )
 756+ .map( function() {
 757+ return $.trim( $( this ).text() );
 758+ } );
 759+ return cats;
 760+ },
 761+
 762+ /**
 763+ * Check whether a passed category is present in the DOM.
 764+ *
 765+ * @return {Boolean}
 766+ */
 767+ containsCat: function( cat ) {
 768+ cat = $.ucFirst( cat );
 769+ return this.getCats().filter( function() {
 770+ return $.ucFirst( this ) === cat;
 771+ } ).length !== 0;
 772+ },
 773+
 774+ /**
 775+ * Execute or queue an category delete.
 776+ *
 777+ * @param $link {jQuery}
 778+ * @param category
 779+ * @return ?
 780+ */
 781+ handleCategoryDelete: function( $link, category ) {
 782+ var categoryRegex = buildRegex( category, true ),
 783+ that = this;
 784+
 785+ that.doConfirmEdit({
 786+ modFn: function( oldText ) {
 787+ var newText = that.runHooks( oldText, 'beforeDelete', category );
 788+ newText = newText.replace( categoryRegex, '' );
 789+
 790+ if ( newText === oldText ) {
 791+ that.showError( mw.msg( 'ajax-remove-category-error' ) );
754792 return false;
755793 }
756794
757 - return _runHooks ( newText, 'afterDelete', category );
 795+ return that.runHooks( newText, 'afterDelete', category );
758796 },
759 - summary,
760 - shortSummary,
761 - function( unsaved ) {
 797+ actionSummary: mw.msg( 'ajax-remove-category-summary', category ),
 798+ shortSummary: '-[[' + new mw.Title( category, catNsId ) + ']]',
 799+ doneFn: function( unsaved ) {
762800 if ( unsaved ) {
763801 $link.addClass( 'mw-removed-category' );
764802 } else {
765803 $link.parent().remove();
766804 }
767805 },
768 - $link,
769 - 'delete'
770 - );
771 - };
772 -
 806+ $link: $link,
 807+ action: 'delete'
 808+ });
 809+ },
773810
774811 /**
775 - * Open a dismissable error dialog
 812+ * Takes a category link element
 813+ * and strips all data from it.
776814 *
777 - * @param string str The error description
 815+ * @param $link {jQuery}
 816+ * @param del {Boolean}
 817+ * @param dontRestoreText {Boolean}
 818+ * @return ?
778819 */
779 - _showError = function( str ) {
780 - var oldDialog = $( '.mw-ajax-confirm-dialog' );
781 - that._removeProgressIndicator( oldDialog );
782 - oldDialog.dialog( 'close' );
783 -
784 - var dialog = $( '<div/>' );
785 - dialog.text( str );
 820+ resetCatLink: function( $link, del, dontRestoreText ) {
 821+ $link.removeClass( 'mw-removed-category mw-added-category mw-changed-category' );
 822+ var data = $link.data();
786823
787 - mw.util.$content.append( dialog );
 824+ if ( typeof data.stashIndex === 'number' ) {
 825+ this.removeStashItem( data.stashIndex );
 826+ }
 827+ if ( del ) {
 828+ $link.parent.remove();
 829+ return;
 830+ }
 831+ if ( data.origCat && !dontRestoreText ) {
 832+ $link.text( data.origCat );
 833+ $link.attr( 'href', catUrl( data.origCat ) );
 834+ }
788835
789 - var buttons = { };
790 - buttons[mw.msg( 'ajax-confirm-ok' )] = function( e ) {
791 - dialog.dialog( 'close' );
792 - };
793 - var dialogOptions = {
794 - 'buttons' : buttons,
795 - 'AutoOpen' : true,
796 - 'title' : mw.msg( 'ajax-error-title' )
797 - };
 836+ $link.removeData();
798837
799 - dialog.dialog( dialogOptions );
 838+ // Readd static.
 839+ $link.data( {
 840+ saveButton: data.saveButton,
 841+ deleteButton: data.deleteButton,
 842+ editButton: data.editButton
 843+ } );
 844+ },
800845
801 - // Close on enter
802 - dialog.keyup( function( e ) {
803 - if ( e.keyCode == 13 ) dialog.dialog( 'close' );
804 - });
805 - };
806 -
807846 /**
808 - * Manufacture iconed button, with or without text
809 - *
810 - * @param string icon The icon class.
811 - * @param string title Title attribute.
812 - * @param string className (optional) Additional classes to be added to the button.
813 - * @param string text (optional) Text of button.
814 - *
815 - * @return jQueryObject The button
 847+ * Do the actual edit.
 848+ * Gets token & text from api, runs it through fn
 849+ * and saves it with summary.
 850+ * @param page {String} Pagename
 851+ * @param fn {Function} edit function
 852+ * @param summary {String}
 853+ * @param doneFn {String} Callback after all is done
816854 */
817 - _createButton = function( icon, title, className, text ){
818 - // We're adding a zero width space for IE7, it's got problems with empty nodes apparently
819 - var $button = $( '<a>' ).addClass( className || '' )
820 - .attr( 'title', title ).html( '&#8203;' );
 855+ doEdit: function( page, fn, summary, doneFn ) {
 856+ // Get an edit token for the page.
 857+ var getTokenVars = {
 858+ action: 'query',
 859+ prop: 'info|revisions',
 860+ intoken: 'edit',
 861+ titles: page,
 862+ rvprop: 'content|timestamp',
 863+ format: 'json'
 864+ }, that = this;
821865
822 - if ( text ) {
823 - var $icon = $( '<span>' ).addClass( 'icon ' + icon ).html( '&#8203;' );
824 - $button.addClass( 'icon-parent' ).append( $icon ).append( text );
825 - } else {
826 - $button.addClass( 'icon ' + icon );
827 - }
828 - return $button;
829 - };
 866+ $.post(
 867+ mw.util.wikiScript( 'api' ),
 868+ getTokenVars,
 869+ function( reply ) {
 870+ var infos = reply.query.pages;
 871+ $.each( infos, function( pageid, data ) {
 872+ var token = data.edittoken;
 873+ var timestamp = data.revisions[0].timestamp;
 874+ var oldText = data.revisions[0]['*'];
830875
 876+ // Replace all nowiki and comments with unique keys
 877+ var key = mw.user.generateId();
 878+ var nowiki = [];
 879+ oldText = replaceNowikis( oldText, key, nowiki );
 880+
 881+ // Then do the changes
 882+ var newText = fn( oldText );
 883+ if ( newText === false ) {
 884+ return;
 885+ }
 886+
 887+ // And restore them back
 888+ newText = restoreNowikis( newText, key, nowiki );
 889+
 890+ var postEditVars = {
 891+ action: 'edit',
 892+ title: page,
 893+ text: newText,
 894+ summary: summary,
 895+ token: token,
 896+ basetimestamp: timestamp,
 897+ format: 'json'
 898+ };
 899+
 900+ $.post( mw.util.wikiScript( 'api' ), postEditVars, doneFn, 'json' )
 901+ .error( function( xhr, text, error ) {
 902+ that.showError( mw.msg( 'ajax-api-error', text, error ) );
 903+ });
 904+ } );
 905+ },
 906+ 'json'
 907+ ).error( function( xhr, text, error ) {
 908+ that.showError( mw.msg( 'ajax-api-error', text, error ) );
 909+ } );
 910+ },
 911+
831912 /**
832 - * Append edit and remove buttons to a given category link
 913+ * This gets called by all action buttons
 914+ * Displays a dialog to confirm the action
 915+ * Afterwards do the actual edit.
833916 *
834 - * @param DOMElement element Anchor element, to which the buttons should be appended.
 917+ * @param props {Object}:
 918+ * - modFn {Function} text-modifying function
 919+ * - actionSummary {String} Changes done
 920+ * - shortSummary {String} Changes, short version
 921+ * - doneFn {Function} callback after everything is done
 922+ * - $link {jQuery}
 923+ * - action
 924+ * @return {mw.ajaxCategories}
835925 */
836 - _createCatButtons = function( $element ) {
837 - // Create remove & edit buttons
838 - var deleteButton = _createButton( 'icon-close', mw.msg( 'ajax-remove-category' ) );
839 - var editButton = _createButton( 'icon-edit', mw.msg( 'ajax-edit-category' ) );
 926+ doConfirmEdit: function( props ) {
 927+ var summaryHolder, reasonBox, dialog, submitFunction,
 928+ buttons = {},
 929+ dialogOptions = {
 930+ AutoOpen: true,
 931+ buttons: buttons,
 932+ width: 450
 933+ },
 934+ that = this;
840935
841 - //Not yet used
842 - var saveButton = _createButton( 'icon-tick', mw.msg( 'ajax-confirm-save' ) ).hide();
 936+ // Check whether to use multiEdit mode:
 937+ if ( this.options.multiEdit && props.action !== 'all' ) {
843938
844 - deleteButton.click( that._handleDeleteLink );
845 - editButton.click( that._createEditInterface );
 939+ // Stash away
 940+ props.$link
 941+ .data( 'stashIndex', this.stash.fns.length )
 942+ .data( 'summary', props.actionSummary );
846943
847 - $element.after( deleteButton ).after( editButton );
 944+ this.stash.summaries.push( props.actionSummary );
 945+ this.stash.shortSum.push( props.shortSummary );
 946+ this.stash.fns.push( props.modFn );
848947
849 - //Save references to all links and buttons
850 - $element.data({
851 - saveButton : saveButton,
852 - deleteButton: deleteButton,
853 - editButton : editButton
854 - });
855 - editButton.data({
856 - link : $element
857 - });
858 - };
 948+ this.saveAllButton.show();
 949+ this.cancelAllButton.show();
859950
860 - /**
861 - * Create the UI
862 - */
863 - this.setup = function() {
864 - // Could be set by gadgets like HotCat etc.
865 - if ( mw.config.get( 'disableAJAXCategories' ) ) {
866 - return false;
 951+ // Clear input field after action
 952+ that.addContainer.find( '.mw-addcategory-input' ).val( '' );
 953+
 954+ // This only does visual changes, fire done and return.
 955+ props.doneFn( true );
 956+ return this;
867957 }
868 - // Only do it for articles.
869 - if ( !mw.config.get( 'wgIsArticle' ) ) return;
870958
871 - // Create [Add Category] link
872 - var addLink = _createButton( 'icon-add',
873 - mw.msg( 'ajax-add-category' ),
874 - 'mw-ajax-addcategory',
875 - mw.msg( 'ajax-add-category' )
876 - );
877 - addLink.click( function() {
878 - $( this ).nextAll().toggle().filter( '.mw-addcategory-input' ).focus();
879 - });
880 -
 959+ // Summary of the action to be taken
 960+ summaryHolder = $( '<p>' )
 961+ .html( '<strong>' + mw.msg( 'ajax-category-question' ) + '</strong><br/>' + props.actionSummary );
881962
882 - // Create add category prompt
883 - _addContainer = that._makeSuggestionBox( '', that._handleAddLink, mw.msg( 'ajax-add-category-submit' ) );
884 - _addContainer.children().hide();
 963+ // Reason textbox.
 964+ reasonBox = $( '<input type="text" size="45"></input>' )
 965+ .addClass( 'mw-ajax-confirm-reason' );
885966
886 - _addContainer.prepend( addLink );
 967+ // Produce a confirmation dialog
 968+ dialog = $( '<div>' )
 969+ .addClass( 'mw-ajax-confirm-dialog' )
 970+ .attr( 'title', mw.msg( 'ajax-confirm-title' ) )
 971+ .append( summaryHolder )
 972+ .append( reasonBox );
887973
888 - // Create edit & delete link for each category.
889 - $( '#catlinks li a' ).each( function() {
890 - _createCatButtons( $( this ) );
891 - });
 974+ // Submit button
 975+ submitFunction = function() {
 976+ that.addProgressIndicator( dialog );
 977+ that.doEdit(
 978+ mw.config.get( 'wgPageName' ),
 979+ props.modFn,
 980+ props.shortSummary + ': ' + reasonBox.val(),
 981+ function() {
 982+ props.doneFn();
892983
893 - options.$containerNormal.append( _addContainer );
 984+ // Clear input field after successful edit
 985+ that.addContainer.find( '.mw-addcategory-input' ).val( '' );
894986
895 - //TODO Make more clickable
896 - _saveAllButton = _createButton( 'icon-tick',
897 - mw.msg( 'ajax-confirm-save-all' ),
898 - '',
899 - mw.msg( 'ajax-confirm-save-all' )
900 - );
901 - _cancelAllButton = _createButton( 'icon-close',
902 - mw.msg( 'ajax-cancel-all' ),
903 - '',
904 - mw.msg( 'ajax-cancel-all' )
905 - );
906 - _saveAllButton.click( that._handleStashedCategories ).hide();
907 - _cancelAllButton.click( function() { that.resetAll( false ); } ).hide();
908 - options.$containerNormal.append( _saveAllButton ).append( _cancelAllButton );
909 - options.$container.append( _addContainer );
910 - };
 987+ dialog.dialog( 'close' );
 988+ that.removeProgressIndicator( dialog );
 989+ }
 990+ );
 991+ };
911992
912 - _stash = {
913 - summaries : [],
914 - shortSum : [],
915 - fns : []
916 - };
917 - _removeStashItem = function( i ) {
918 - if ( typeof i != "number" ) {
 993+ buttons[mw.msg( 'ajax-confirm-save' )] = submitFunction;
 994+
 995+ dialog.dialog( dialogOptions ).keyup( function( e ) {
 996+ // Close on enter
 997+ if ( e.keyCode === 13 ) {
 998+ submitFunction();
 999+ }
 1000+ } );
 1001+
 1002+ return this;
 1003+ },
 1004+
 1005+ /**
 1006+ * @param index {Number|jQuery} Stash index or jQuery object of stash item.
 1007+ * @return {mw.ajaxCategories}
 1008+ */
 1009+ removeStashItem: function( i ) {
 1010+ if ( typeof i !== 'number' ) {
9191011 i = i.data( 'stashIndex' );
9201012 }
921 - delete _stash.fns[i];
922 - delete _stash.summaries[i];
923 - if ( $.isEmpty( _stash.fns ) ) {
924 - _stash.fns = [];
925 - _stash.summaries = [];
926 - _stash.shortSum = [];
927 - _saveAllButton.hide();
928 - _cancelAllButton.hide();
 1013+
 1014+ try {
 1015+ delete this.stash.fns[i];
 1016+ delete this.stash.summaries[i];
 1017+ } catch(e) {}
 1018+
 1019+ if ( $.isEmpty( this.stash.fns ) ) {
 1020+ this.stash.fns = [];
 1021+ this.stash.summaries = [];
 1022+ this.stash.shortSum = [];
 1023+ this.saveAllButton.hide();
 1024+ this.cancelAllButton.hide();
9291025 }
930 - };
931 - _hooks = {
932 - beforeAdd : [],
933 - beforeChange : [],
934 - beforeDelete : [],
935 - afterAdd : [],
936 - afterChange : [],
937 - afterDelete : []
938 - };
939 - _runHooks = function( oldtext, type, category, categoryNew ) {
940 - // No hooks registered
941 - if ( !_hooks[type] ) {
942 - return oldtext;
943 - } else {
944 - for ( var i = 0; i < _hooks[type].length; i++ ) {
945 - oldtext = _hooks[type][i]( oldtext, category, categoryNew );
946 - if ( oldtext === false ) {
947 - _showError( mw.msg( 'ajax-category-hook-error', category ) );
948 - return;
949 - }
950 - }
951 - return oldtext;
 1026+ return this;
 1027+ },
 1028+
 1029+ /**
 1030+ * Reset all data from the category links and the stash.
 1031+ *
 1032+ * @param del {Boolean} Delete any category links with .mw-removed-category
 1033+ * @return {mw.ajaxCategories}
 1034+ */
 1035+ resetAll: function( del ) {
 1036+ var $links = this.options.$container.find( this.options.categoryLinkSelector ),
 1037+ $del = $([]),
 1038+ that = this;
 1039+
 1040+ if ( del ) {
 1041+ $del = $links.filter( '.mw-removed-category' ).parent();
9521042 }
953 - };
 1043+
 1044+ $links.each( function() {
 1045+ that.resetCatLink( $( this ), false, del );
 1046+ } );
 1047+
 1048+ $del.remove();
 1049+
 1050+ this.options.$container.find( '#mw-hidden-catlinks' ).remove();
 1051+
 1052+ return this;
 1053+ },
 1054+
9541055 /**
9551056 * Add hooks
9561057 * Currently available: beforeAdd, beforeChange, beforeDelete,
@@ -958,18 +1059,74 @@
9591060 *
9601061 * @param string type Type of hook to add
9611062 * @param function fn Hook function. The following vars are passed to it:
962 - * 1. oldtext: The wikitext before the hook
963 - * 2. category: The deleted, added, or changed category
964 - * 3. (only for beforeChange/afterChange): newcategory
 1063+ * 1. oldtext: The wikitext before the hook
 1064+ * 2. category: The deleted, added, or changed category
 1065+ * 3. (only for beforeChange/afterChange): newcategory
9651066 */
966 - this.addHook = function( type, fn ) {
967 - if ( !_hooks[type] || !$.isFunction( fn ) ) {
 1067+ addHook: function( type, fn ) {
 1068+ if ( !this.hooks[type] || !$.isFunction( fn ) ) {
9681069 return;
9691070 }
9701071 else {
971 - hooks[type].push( fn );
 1072+ this.hooks[type].push( fn );
9721073 }
973 - };
 1074+ },
 1075+
 1076+
 1077+ /**
 1078+ * Open a dismissable error dialog
 1079+ *
 1080+ * @param string str The error description
 1081+ */
 1082+ showError: function( str ) {
 1083+ var oldDialog = $( '.mw-ajax-confirm-dialog' ),
 1084+ buttons = {},
 1085+ dialogOptions = {
 1086+ buttons: buttons,
 1087+ AutoOpen: true,
 1088+ title: mw.msg( 'ajax-error-title' )
 1089+ };
 1090+
 1091+ this.removeProgressIndicator( oldDialog );
 1092+ oldDialog.dialog( 'close' );
 1093+
 1094+ var dialog = $( '<div>' ).text( str );
 1095+
 1096+ mw.util.$content.append( dialog );
 1097+
 1098+ buttons[mw.msg( 'ajax-confirm-ok' )] = function( e ) {
 1099+ dialog.dialog( 'close' );
 1100+ };
 1101+
 1102+ dialog.dialog( dialogOptions ).keyup( function( e ) {
 1103+ if ( e.keyCode === 13 ) {
 1104+ dialog.dialog( 'close' );
 1105+ }
 1106+ } );
 1107+ },
 1108+
 1109+ /**
 1110+ * @param oldtext
 1111+ * @param type
 1112+ * @param category
 1113+ * @param categoryNew
 1114+ * @return oldtext
 1115+ */
 1116+ runHooks: function( oldtext, type, category, categoryNew ) {
 1117+ // No hooks registered
 1118+ if ( !this.hooks[type] ) {
 1119+ return oldtext;
 1120+ } else {
 1121+ for ( var i = 0; i < this.hooks[type].length; i++ ) {
 1122+ oldtext = this.hooks[type][i]( oldtext, category, categoryNew );
 1123+ if ( oldtext === false ) {
 1124+ this.showError( mw.msg( 'ajax-category-hook-error', category ) );
 1125+ return;
 1126+ }
 1127+ }
 1128+ return oldtext;
 1129+ }
 1130+ }
9741131 };
9751132
976 -} )( jQuery );
\ No newline at end of file
 1133+} )( jQuery );
Index: trunk/phase3/resources/mediawiki.page/mediawiki.page.ajaxCategories.init.js
@@ -1 +1,6 @@
2 -jQuery( document ).ready( new mw.ajaxCategories().setup );
 2+mw.page.ajaxCategories = new mw.ajaxCategories();
 3+jQuery( document ).ready( function(){
 4+ // Seperate function for call to prevent jQuery
 5+ // from executing it in the document context.
 6+ mw.page.ajaxCategories.setup();
 7+} );
Index: trunk/phase3/resources/mediawiki.page/images/AJAXCategorySprite.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Index: trunk/phase3/resources/mediawiki.page/images/ajaxcat-error-hover.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Property changes on: trunk/phase3/resources/mediawiki.page/images/ajaxcat-error-hover.png
___________________________________________________________________
Added: svn:mime-type
38 + image/png
Index: trunk/phase3/resources/mediawiki.page/images/ajaxcat-tick.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Property changes on: trunk/phase3/resources/mediawiki.page/images/ajaxcat-tick.png
___________________________________________________________________
Added: svn:mime-type
49 + image/png
Index: trunk/phase3/resources/mediawiki.page/images/ajaxcat-tick-hover.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Property changes on: trunk/phase3/resources/mediawiki.page/images/ajaxcat-tick-hover.png
___________________________________________________________________
Added: svn:mime-type
510 + image/png
Index: trunk/phase3/resources/mediawiki.page/images/ajaxcat-close.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Property changes on: trunk/phase3/resources/mediawiki.page/images/ajaxcat-close.png
___________________________________________________________________
Added: svn:mime-type
611 + image/png
Index: trunk/phase3/resources/mediawiki.page/images/ajaxcat-edit.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Property changes on: trunk/phase3/resources/mediawiki.page/images/ajaxcat-edit.png
___________________________________________________________________
Added: svn:mime-type
712 + image/png
Index: trunk/phase3/resources/mediawiki.page/images/ajaxcat-close-hover.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Property changes on: trunk/phase3/resources/mediawiki.page/images/ajaxcat-close-hover.png
___________________________________________________________________
Added: svn:mime-type
813 + image/png
Index: trunk/phase3/resources/mediawiki.page/images/ajaxcat-edit-hover.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Property changes on: trunk/phase3/resources/mediawiki.page/images/ajaxcat-edit-hover.png
___________________________________________________________________
Added: svn:mime-type
914 + image/png
Index: trunk/phase3/resources/mediawiki.page/images/ajaxcat-add.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Property changes on: trunk/phase3/resources/mediawiki.page/images/ajaxcat-add.png
___________________________________________________________________
Added: svn:mime-type
1015 + image/png
Index: trunk/phase3/resources/mediawiki.page/images/ajaxcat-add-hover.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Property changes on: trunk/phase3/resources/mediawiki.page/images/ajaxcat-add-hover.png
___________________________________________________________________
Added: svn:mime-type
1116 + image/png
Index: trunk/phase3/resources/mediawiki.page/images/ajaxcat-error.png
Cannot display: file marked as a binary type.
svn:mime-type = image/png
Property changes on: trunk/phase3/resources/mediawiki.page/images/ajaxcat-error.png
___________________________________________________________________
Added: svn:mime-type
1217 + image/png

Follow-up revisions

RevisionCommit summaryAuthorDate
r93373Fu r93351: remove extra spacenikerabbit06:41, 28 July 2011
r94268ajaxCategories fixes based on review in r93351 CR:...krinkle19:01, 11 August 2011
r94270Solve undefined-message problem by removing it all together. I've moved the ....krinkle19:14, 11 August 2011
r94338more ajaxCategories fixes based on review in r93351 CR...krinkle11:35, 12 August 2011
r95870Fix Uncaught TypeError: Cannot call method 'substr' of undefined...krinkle14:37, 31 August 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r92288ajaxCategories fixes:...krinkle19:25, 15 July 2011

Comments

#Comment by Krinkle (talk | contribs)   16:58, 29 July 2011

Since many functions have been moved out of the constructor (for performance and readability) into either the prototype reference on the bottom or into a local function experession, the diff is a bit hard to review. I suggest reviewing this as a file rather than a diff.

#Comment by Catrope (talk | contribs)   11:30, 11 August 2011

Reviewing the entire file, adding comments here in lieu.

 		if ( s !== undefined ) {

Since s.replace() can fail even on things that aren't undefined, shouldn't you really be checking for typeof s == 'string' ?

+					title = page.title.split( ':', 2 )[1];

Eww. This is kind of hackish and works only because all the returned titles are in the category namespace. Would the overhead of using mw.Title be bad?

+	 * @param id

Undocumented, as is pretty much everything else in that function BTW.

 			text = text.replace( matches[i], id + i + '-' );

This will do fun unexpected things if id happens to be a Number, you may want to force it to a string. Also, why is there a dash at the end? This whole function doesn't make much sense to me.

 	 * Makes regex string caseinsensitive.

Sounds nice and generic. But then it does this:

+		if ( $.inArray( 14, mw.config.get( 'wgCaseSensitiveNamespaces' ) ) !== -1 ) {

which is not very generic and totally undocumented. Generally, the documentation for this function is terse and would be clearer with an explicit example.

+		categoryNSFragment = $.map( mw.config.get( 'wgNamespaceIds' ), function( id, name ) {
+			if ( id === catNsId ) {
+				return makeCaseInsensitive( $.escapeRE( name ) );
 			}

This seems to rely on the fact that returning null from a $.map() callback removes the element (which is documented but not immediately intuitive), but what's worse is that what it really does is 1) rely on the fact that not returning anything from a function will make it return undefined, which is evil and 2) rely on the fact that $.map() will treat undefined the same as null, which is undocumented as well. Please add an explicit return null with a short comment explaining why you're returning null (so $.map() will remove the element).

+		categoryRegex = '\\[\\[(' + categoryNSFragment + '):' + '[ _]*' + titleFragment + '(\\|[^\\]]*)?\\]\\]';

If you're accounting for spaces after the colon, account for spaces before the colon too:

> echo Title::newFromText('Category __    :  _  Foo')->getPrefixedText();
Category:Foo
 		if ( matchLineBreak ) {
 			categoryRegex += '[ \\t\\r]*\\n?';

So this could make the regex potentially match a bunch of spaces after the link, but not a line break? That's confusing. Document this if it's intended, or fix it if it's not.

+			$button.addClass( 'icon-parent' ).append( $icon ).append( text );

This means text is treated as HTML. Document if intended (or rename the var), fix if not.

I only got down to line 180 or so before lunchtime, will continue after I eat.

#Comment by Catrope (talk | contribs)   14:43, 11 August 2011
+					category = new mw.Title( redirect[0].to )._name;

Are you supposed to be accessing private members of mw.Title like that?

 	/**
-	 * Execute or queue an category add
+	 * Execute or queue an category add.
+	 * @param $link {jQuery}
+	 * @param category
+	 * @param noAppend
+	 * @param exists
+	 * @return {mw.ajaxCategories}

The behavior of this function is barely documented, specifically what it does with $link. In fact, a lot of the internal functions are undocumented, and that's only OK if they're doing trivial things.

+			this.showError( 'Unexpected error occurred. $link undefined.' );

Hardcoded English.

+				$( this )
+					.unbind( 'click' )
+					.click( that.handleDeleteLink )

Could you document what's going on in the click handler here? You're unbinding and rebinding the click handler from inside the click handler and doing some other funky stuff, which one has no chance of understanding unless one is thoroughly familiar with the interface. I think I sort of understand now, after reading it a second time, but some comments would be nice.

Also, whenever you're unbinding event handlers, you should namespace them so you don't end up unbinding someone else's handlers by accident.

			$link.data( 'deleteButton' ).click();

This is just one occurrence, this pattern appears all over the code. Calling click handlers on UI elements to achieve some action is bad form IMO. Instead, make the handler public (or at least visible to the code that needs it), and have other code call it directly.

+		$( '#catlinks li a' ).each( function() {

Isn't this a big faux pas in your own JSPERF guide?

+			$anchor = $( '<a>' )
+				.append( cat )

cat is a category name, so you need .text( cat ) here to escape things properly.

 				if ( matches.length > 1 ) {
 					// The category is duplicated.
 					// Remove all but one match
 					for ( var i = 1; i < matches.length; i++ ) { 
						oldText = oldText.replace( matches[i], '' );
					}
				}
				newText = oldText.replace( categoryRegex, newCategoryString );

This can produce inconsistent results. Imagine what happens if, for instance, matches[2] === matches[0]. This isn't too bad, it's just weird. Performance also isn't ideal, because the string is searched multiple times. I think it would be more intuitive to do something like var done = false; newText = oldText.replace( categoryRegex, function() { if ( !done ) { done = true; return newCategoryString; } else { return ; } } );. That would also avoid the weirdness and the performance concern I described.

	containsCat: function( cat ) {
		cat = $.ucFirst( cat );
		return this.getCats().filter( function() {
			return $.ucFirst( this ) === cat;
		} ).length !== 0;
	},

Using .filter() like that is cute, but in this case inefficient. If there are a hundred categories and the one you're looking for is first, it'll still traverse the other 99. Instead, use .each() to traverse the list. When you find a match, set a function-scope variable to true to indicate you found a match, then return false from the each callback to stop the traversal.

			$link.parent.remove();

That will throw a JS error, because $link.parent is a function. You need $link.parent().remove().

				var infos = reply.query.pages;

This'll cause a JS error if, for some reason, the query element is not present in the response (I'm not sure the API will return such a response in practice; I guess in theory it could happen if there's like a DB error response or something), so program defensively and make sure things exist before you access them.

				$.each( infos, function( pageid, data ) {
					var token = data.edittoken;
					var timestamp = data.revisions[0].timestamp;
					var oldText = data.revisions[0]['*'];

					// Replace all nowiki and comments with unique keys
					var key = mw.user.generateId();
					var nowiki = [];

var, var, var, var, var. And a few more a bit further down in the same function.

			summary = summary.join( '<br/>' );

In this case, summary comes from that.stash.summaries, which is filled with text strings that aren't escaped for HTML, then treated as HTML by this join here. So that's a potential XSS vector, or at least something that scares me.

		summaryHolder = $( '<p>' )
			.html( '<strong>' + mw.msg( 'ajax-category-question' ) + '</strong><br/>' + props.actionSummary );

And here props.actionSummary is unescaped text that's thrown into the HTML, which is scary, as is the fact that the ajax-category-question message is treated as an HTML message for no good reason.

That's all I have.

#Comment by Krinkle (talk | contribs)   19:01, 11 August 2011

I've believe all issues are fixed with r94268.

Here's the points that I've skipped for now that I'd like to give some more thought / discussion:

  • 'Unexpected error occurred. $link undefined' error message should not be hardcoded in English.
  • Calling methods directly instead of triggering .click() on buttons.

Isn't this a big faux pas in your own JSPERF guide?

Yes, it sure is. I didn't introduce it though (It was originally .catlinks span a</code' and changed to #catlinks li a in r92112), but I should've catched it in the rewrite! Fixed now in r94268.

#Comment by Krinkle (talk | contribs)   19:02, 11 August 2011

#Comment by Catrope (talk | contribs)   09:15, 12 August 2011
 		if ( matchLineBreak ) {
 			categoryRegex += '[ \\t\\r]*\\n?';

So this could make the regex potentially match a bunch of spaces after the link, but not a line break? That's confusing. Document this if it's intended, or fix it if it's not.

#Comment by Catrope (talk | contribs)   09:27, 12 August 2011
 		if ( matchLineBreak ) {
 			categoryRegex += '[ \\t\\r]*\\n?';

So this could make the regex potentially match a bunch of spaces after the link, but not a line break? That's confusing. Document this if it's intended, or fix it if it's not.

This one wasn't addressed.

 	/**
-	 * Execute or queue an category add
+	 * Execute or queue an category add.
+	 * @param $link {jQuery}
+	 * @param category
+	 * @param noAppend
+	 * @param exists
+	 * @return {mw.ajaxCategories}

The behavior of this function is barely documented, specifically what it does with $link. In fact, a lot of the internal functions are undocumented, and that's only OK if they're doing trivial things.

This was addressed somewhat, but the magic that happens when $link is an empty jQuery object still isn't documented.

 				if ( matches.length > 1 ) {
 					// The category is duplicated.
 					// Remove all but one match
 					for ( var i = 1; i < matches.length; i++ ) { 
						oldText = oldText.replace( matches[i], '' );
					}
				}
				newText = oldText.replace( categoryRegex, newCategoryString );

This can produce inconsistent results. Imagine what happens if, for instance, matches[2] === matches[0]. This isn't too bad, it's just weird. Performance also isn't ideal, because the string is searched multiple times. I think it would be more intuitive to do something like var done = false; newText = oldText.replace( categoryRegex, function() { if ( !done ) { done = true; return newCategoryString; } else { return ''; } } );. That would also avoid the weirdness and the performance concern I described.

This wasn't addressed either.

				var infos = reply.query.pages;

This'll cause a JS error if, for some reason, the query element is not present in the response (I'm not sure the API will return such a response in practice; I guess in theory it could happen if there's like a DB error response or something), so program defensively and make sure things exist before you access them.

Unaddressed.

		summaryHolder = $( '<p>' )
			.html( '<strong>' + mw.msg( 'ajax-category-question' ) + '</strong><br/>' + props.actionSummary );

And here props.actionSummary is unescaped text that's thrown into the HTML, which is scary, as is the fact that the ajax-category-question message is treated as an HTML message for no good reason.

actionSummaries (renamed to dialogDescriptions) are now always HTML and escaped properly but 1) this isn't documented anywhere (and it must be, or people will do it wrong in the future) and 2) it doesn't address the issue that the ajax-category-question message is treated as HTML for no good reason. HTML messages should be avoided unless they're needed.

#Comment by Krinkle (talk | contribs)   11:36, 12 August 2011

The categoryRegex in the matchLineBreak condition remained unchanged since I'm terrible at regexes and don't know a lot about how the Parser handles category links, – I left the regex the way it was introduced by mdale. I understand the regex to recognize the issue, but don't know how to fix it. Same goes for newText = oldText.replace( categoryRegex, newCategoryString );.

I didn't change the event handler a lot because (together with the culture of triggers events to perform actions and using DOM-inspection to get information), I'd like to get rid of that all together and instead store it in JavaScript (perhaps wgCategories or a local copy of it). That would also make containCat() a lot nicer. Events would then just call the API rather than the events being the API. This will likely require another major refactor of this module, which I don't have time for right now but would love to do later (I don't think it's a requirement for the module though, it works fairly well right now).

Fixed object member access, escaping and documentation about dialogDescription in r94338.

#Comment by Krinkle (talk | contribs)   14:17, 31 August 2011

Test cases: http://www.mediawiki.org/w/index.php?title=Sandbox&oldid=430268

Steps to test this module (todo: Automate this once we have a testing environment for it)

  1. Copy wikitext to your wiki's [[Sandbox]] -page
  2. Try the following tasks After every action check the history/diff and refresh the page to see what it looks like now
    1. Add a new category "Ajax add"
    2. Remove category "Ajax add"
    3. Change "Quux", does it preserve sort key ?
    4. Remove "Bar"
    5. Remove "Foo", does it remove the right one ?
#Comment by Krinkle (talk | contribs)   19:38, 4 September 2011

Marking deferred, moved out of core in r96250.

As shown above, there are too many issues. I have no time to fix them and I didn't create the module in the first place. Aside from bugs, there's also structural problems (like the lack of separation between api-logic and ui-events binding and callbacks, it's a bit of a soup right now)

See also User:Krinkle/Extension review/InlineCategorizer for a summary.

Status & tagging log