r105007 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r105006‎ | r105007 | r105008 >
Date:21:47, 2 December 2011
Author:brion
Status:ok
Tags:
Comment:
Followup r104671: fix regression in jquery.delayedBind() due to change in param processing for jquery's bind()

Problem was per CR comment https://www.mediawiki.org/wiki/Special:Code/MediaWiki/104671#c26836 -- parameter count check changed in upstream jquery.bind(), causing the way we passed params to break if the optional 'data' parameter was left out.

Added QUnit test cases for jquery.delayedBind() with and without the optional 'data' parameter.
Undoes r104671 since the issue it was working around is fixed.
Modified paths:
  • /trunk/phase3/resources/jquery/jquery.collapsibleTabs.js (modified) (history)
  • /trunk/phase3/resources/jquery/jquery.delayedBind.js (modified) (history)
  • /trunk/phase3/tests/qunit/index.html (modified) (history)
  • /trunk/phase3/tests/qunit/suites/resources/jquery/jquery.delayedBind.test.js (added) (history)

Diff [purge]

Index: trunk/phase3/tests/qunit/index.html
@@ -62,6 +62,7 @@
6363 <script src="../../resources/jquery/jquery.byteLength.js"></script>
6464 <script src="../../resources/jquery/jquery.byteLimit.js"></script>
6565 <script src="../../resources/jquery/jquery.colorUtil.js"></script>
 66+ <script src="../../resources/jquery/jquery.delayedBind.js"></script>
6667 <script src="../../resources/jquery/jquery.getAttrs.js"></script>
6768 <script src="../../resources/jquery/jquery.highlightText.js"></script>
6869 <script src="../../resources/jquery/jquery.localize.js"></script>
@@ -91,6 +92,7 @@
9293 <script src="suites/resources/jquery/jquery.byteLength.test.js"></script>
9394 <script src="suites/resources/jquery/jquery.byteLimit.test.js"></script>
9495 <script src="suites/resources/jquery/jquery.colorUtil.test.js"></script>
 96+ <script src="suites/resources/jquery/jquery.delayedBind.test.js"></script>
9597 <script src="suites/resources/jquery/jquery.getAttrs.test.js"></script>
9698 <script src="suites/resources/jquery/jquery.highlightText.test.js"></script>
9799 <script src="suites/resources/jquery/jquery.localize.test.js"></script>
Index: trunk/phase3/tests/qunit/suites/resources/jquery/jquery.delayedBind.test.js
@@ -0,0 +1,41 @@
 2+test('jquery.delayedBind with data option', function() {
 3+ var $fixture = $('<div>').appendTo('body'),
 4+ data = { magic: "beeswax" },
 5+ delay = 50;
 6+
 7+ $fixture.delayedBind(delay, 'testevent', data, function(event) {
 8+ start(); // continue!
 9+ ok(true, 'testevent fired');
 10+ ok(event.data === data, 'data is passed through delayedBind');
 11+ });
 12+
 13+ expect(2);
 14+ stop(); // async!
 15+
 16+ // We'll trigger it thrice, but it should only happen once.
 17+ $fixture.trigger('testevent', {});
 18+ $fixture.trigger('testevent', {});
 19+ $fixture.trigger('testevent', {});
 20+ $fixture.trigger('testevent', {});
 21+});
 22+
 23+test('jquery.delayedBind without data option', function() {
 24+ var $fixture = $('<div>').appendTo('body'),
 25+ data = { magic: "beeswax" },
 26+ delay = 50;
 27+
 28+ $fixture.delayedBind(delay, 'testevent', function(event) {
 29+ start(); // continue!
 30+ ok(true, 'testevent fired');
 31+ });
 32+
 33+ expect(1);
 34+ stop(); // async!
 35+
 36+ // We'll trigger it thrice, but it should only happen once.
 37+ $fixture.trigger('testevent', {});
 38+ $fixture.trigger('testevent', {});
 39+ $fixture.trigger('testevent', {});
 40+ $fixture.trigger('testevent', {});
 41+});
 42+
Property changes on: trunk/phase3/tests/qunit/suites/resources/jquery/jquery.delayedBind.test.js
___________________________________________________________________
Added: svn:eol-style
143 + native
Index: trunk/phase3/resources/jquery/jquery.collapsibleTabs.js
@@ -24,7 +24,7 @@
2525 // if we haven't already bound our resize hanlder, bind it now
2626 if( !$.collapsibleTabs.boundEvent ) {
2727 $( window )
28 - .delayedBind( '500', 'resize', null, function( ) { $.collapsibleTabs.handleResize(); } );
 28+ .delayedBind( '500', 'resize', function( ) { $.collapsibleTabs.handleResize(); } );
2929 }
3030 // call our resize handler to setup the page
3131 $.collapsibleTabs.handleResize();
Index: trunk/phase3/resources/jquery/jquery.delayedBind.js
@@ -19,6 +19,11 @@
2020 * @param callback Function to call
2121 */
2222 delayedBind: function( timeout, event, data, callback ) {
 23+ if ( arguments.length == 3 ) {
 24+ // Shift optional parameter down
 25+ callback = data;
 26+ data = undefined;
 27+ }
2328 var encEvent = encodeEvent( event );
2429 return this.each( function() {
2530 var that = this;

Follow-up revisions

RevisionCommit summaryAuthorDate
r107913MFT r105007reedy18:03, 3 January 2012
r107957Merge r107913 (merges r105007 from trunk) and r107924 (merges r106992 from tr...reedy22:57, 3 January 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r104671(bug 31212) History tab not collapsed when the screen is narrow. Patch by Mic...catrope13:16, 30 November 2011

Status & tagging log