r92238 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r92237‎ | r92238 | r92239 >
Date:11:12, 15 July 2011
Author:diebuche
Status:reverted (Comments)
Tags:
Comment:
AjaxCategories: move self init to mw.util.init; start with qunit
Modified paths:
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)
  • /trunk/phase3/maintenance/language/messages.inc (modified) (history)
  • /trunk/phase3/resources/Resources.php (modified) (history)
  • /trunk/phase3/resources/mediawiki.page/mediawiki.page.ajaxCategories.js (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.util.js (modified) (history)
  • /trunk/phase3/tests/qunit/index.html (modified) (history)
  • /trunk/phase3/tests/qunit/suites/resources/mediawiki.page (added) (history)
  • /trunk/phase3/tests/qunit/suites/resources/mediawiki.page/mediawiki.page.ajaxCategories.js (added) (history)

Diff [purge]

Index: trunk/phase3/maintenance/language/messages.inc
@@ -3473,6 +3473,9 @@
34743474 'ajax-add-category-summary',
34753475 'ajax-edit-category-summary',
34763476 'ajax-remove-category-summary',
 3477+ 'ajax-add-category-question',
 3478+ 'ajax-edit-category-question',
 3479+ 'ajax-remove-category-question',
34773480 'ajax-confirm-actionsummary',
34783481 'ajax-error-title',
34793482 'ajax-error-dismiss',
Index: trunk/phase3/tests/qunit/index.html
@@ -47,9 +47,11 @@
4848 <script src="../../resources/jquery/jquery.localize.js"></script>
4949 <script src="../../resources/jquery/jquery.tabIndex.js"></script>
5050 <script src="../../resources/jquery/jquery.tablesorter.js"></script>
 51+ <script src="../../resources/jquery/jquery.suggestions.js"></script>
5152 <script src="../../resources/mediawiki/mediawiki.Title.js"></script>
5253 <script src="../../resources/mediawiki.special/mediawiki.special.js"></script>
5354 <script src="../../resources/mediawiki.special/mediawiki.special.recentchanges.js"></script>
 55+ <script src="../../resources/mediawiki.page/mediawiki.page.ajaxCategories.js"></script>
5456
5557 <!-- QUnit: Load framework -->
5658 <link rel="stylesheet" href="../../resources/jquery/jquery.qunit.css" />
@@ -75,6 +77,7 @@
7678 <script src="suites/resources/jquery/jquery.tablesorter.test.js" charset="UTF-8"></script>
7779 <script src="suites/resources/mediawiki/mediawiki.Title.js"></script>
7880 <script src="suites/resources/mediawiki.special/mediawiki.special.recentchanges.js"></script>
 81+ <!--<script src="suites/resources/mediawiki.page/mediawiki.page.ajaxCategories.js"></script>-->
7982 </head>
8083 <body>
8184 <h1 id="qunit-header">MediaWiki JavaScript Test Suite</h1>
@@ -85,7 +88,7 @@
8689 <h2 id="qunit-userAgent"></h2>
8790 <ol id="qunit-tests"></ol>
8891
89 -<!-- Scripts inserting stuff here shall remove it themselfs! -->
 92+<!-- Scripts inserting stuff here shall remove it themselves! -->
9093 <div id="content"></div>
9194 </body>
9295 </html>
Index: trunk/phase3/tests/qunit/suites/resources/mediawiki.page/mediawiki.page.ajaxCategories.js
@@ -0,0 +1,36 @@
 2+(function( mw ) {
 3+
 4+module( 'mediawiki.page.ajaxCategories.js' );
 5+mw.config.set( 'wgNamespaceIds', {'category' : 14} );
 6+test( '-- Initial check', function() {
 7+ expect(1);
 8+ ok( mw.ajaxCategories, 'mw.ajaxCategories defined' );
 9+});
 10+
 11+/**
 12+ * Create a category list like the one found below articles.
 13+ * @param {String[]} categories
 14+ * @return jQuery
 15+ */
 16+var listCreate = function( categories ) {
 17+ var $container = $('<div id="catlinks" class="catlinks"><div id="mw-normal-catlinks"><ul></ul></div></div>'),
 18+ $ul = $container.find('ul');
 19+ $.each( categories, function(i, str) {
 20+ var $li = $('<li>');
 21+ $li.text(str).appendTo($ul);
 22+ });
 23+
 24+ return $container;
 25+};
 26+catList1 = ['Earth satellites', 'Space stations', 'astronauts'];
 27+
 28+test( 'Testing containsCat', function() {
 29+ expect(1);
 30+ $( 'body' ).append( listCreate(catList1) );
 31+ mw.ajaxCategories.setup();
 32+ var ret = mw.ajaxCategories.containsCat('Earth satellites')
 33+
 34+ equal(ret, true);
 35+});
 36+
 37+})( mediaWiki );
\ No newline at end of file
Property changes on: trunk/phase3/tests/qunit/suites/resources/mediawiki.page/mediawiki.page.ajaxCategories.js
___________________________________________________________________
Added: svn:eol-style
138 + native
Index: trunk/phase3/languages/messages/MessagesEn.php
@@ -4602,6 +4602,9 @@
46034603 'ajax-add-category-summary' => 'Add category "$1"',
46044604 'ajax-edit-category-summary' => 'Change category "$1" to "$2"',
46054605 'ajax-remove-category-summary' => 'Remove category "$1"',
 4606+'ajax-add-category-question' => 'Why do you want to add category "$1"?',
 4607+'ajax-edit-category-question' => 'Why do you want to change category "$1" to "$2"?',
 4608+'ajax-remove-category-question'=> 'Why do you want to remove category "$1"?',
46064609 'ajax-confirm-actionsummary' => 'Action to take:',
46074610 'ajax-error-title' => 'Error',
46084611 'ajax-error-dismiss' => 'OK',
Index: trunk/phase3/resources/Resources.php
@@ -508,6 +508,9 @@
509509 'ajax-add-category-summary',
510510 'ajax-remove-category-summary',
511511 'ajax-edit-category-summary',
 512+ 'ajax-add-category-question',
 513+ 'ajax-edit-category-question',
 514+ 'ajax-remove-category-question',
512515 'ajax-confirm-actionsummary',
513516 'ajax-error-title',
514517 'ajax-error-dismiss',
Index: trunk/phase3/resources/mediawiki/mediawiki.util.js
@@ -99,6 +99,12 @@
100100 $tocToggleLink.click();
101101 }
102102 }
 103+ /* Ajax Categories */
 104+ if ( mw.ajaxCategories ) {
 105+ // Execute only on doc.ready, so that everyone
 106+ // gets a chance to set mw.config.set('disableAJAXCategories')
 107+ mw.ajaxCategories.setup()
 108+ }
103109 } );
104110
105111 return true;
Index: trunk/phase3/resources/mediawiki.page/mediawiki.page.ajaxCategories.js
@@ -85,7 +85,7 @@
8686 // strip out bad characters
8787 cat = _stripIllegals ( cat );
8888
89 - if ( $.isEmpty( cat ) || _containsCat( cat ) ) {
 89+ if ( $.isEmpty( cat ) || this.containsCat( cat ) ) {
9090 return;
9191 }
9292
@@ -157,7 +157,7 @@
158158 *
159159 * @return boolean True for exists
160160 */
161 - _containsCat = function ( cat ) {
 161+ this.containsCat = function ( cat ) {
162162 return _getCats().filter( function() { return $.ucFirst(this) == $.ucFirst(cat); } ).length !== 0;
163163 };
164164
@@ -449,7 +449,7 @@
450450 var category = $( this ).parent().find( '.mw-addcategory-input' ).val();
451451 category = $.ucFirst( category );
452452
453 - if ( _containsCat(category) ) {
 453+ if ( this.containsCat(category) ) {
454454 _showError( mw.msg( 'ajax-category-already-present', category ) );
455455 return;
456456 }
@@ -463,6 +463,7 @@
464464 },
465465 summary,
466466 function() {
 467+ $container.find( '#mw-normal-catlinks>.mw-addcategory-prompt' ).toggle();
467468 _insertCatDOM( category, false );
468469 }
469470 );
@@ -687,8 +688,4 @@
688689 // Now make a new version
689690 mw.ajaxCategories = new ajaxCategories();
690691
691 -// Executing only on doc.ready, so that everyone
692 -// gets a chance to set mw.config.set('disableAJAXCategories')
693 -$( document ).ready( mw.ajaxCategories.setup() );
694 -
695692 } )( jQuery, mediaWiki );
\ No newline at end of file

Follow-up revisions

RevisionCommit summaryAuthorDate
r92253Revert r92238: partial addition of broken test cases to qunit test suite; loa...brion17:29, 15 July 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   17:17, 15 July 2011

Test failures on all browsers: http://toolserver.org/~krinkle/testswarm/job/223/

The tests don't appear to actually get run...

#Comment by Brion VIBBER (talk | contribs)   17:25, 15 July 2011

Looks like the test isn't being loaded (but is somehow getting defined...?) but the base module to be tested *is* being loaded... and erroring out because it assumes that config vars are available at load time:

namespaceIds is null
[http://stormcloud.local/trunk/resources/mediawiki.page/mediawiki.page.ajaxCategories.js http://stormcloud.local/trunk/resources/mediawiki.page/mediawiki.page.ajaxCategories.js]
Line 28

That doesn't get set until the test gets loaded, which never happens because the test's script is commented out in index.html.

#Comment by Brion VIBBER (talk | contribs)   17:29, 15 July 2011

Reverted in r92253; please make sure the tests work and don't introduce errors before committing.

#Comment by DieBuche (talk | contribs)   18:01, 15 July 2011

That testswarm parses uncommented files, which qunit standalone ignores is not an error on my side.

Status & tagging log