mw.hook: Make hook.fire actually chainable
authorTimo Tijhof <krinklemail@gmail.com>
Fri, 4 Apr 2014 01:35:33 +0000 (18:35 -0700)
committerOri.livneh <ori@wikimedia.org>
Fri, 4 Apr 2014 18:33:50 +0000 (18:33 +0000)
It was being called without being bound to the mw.hook instance,
as such the (detachable) callbackList.fireWith method (which
returns its given 'this') returned the callbackList instance
instead of the mw.hook object literal.

The tests weren't catching it because it was just checking one
can call .fire() without it crashing, but it was in fact calling
list.fire instead of hook.fire (which unlike #add and #remove are
not compatible, and we might add new methods later on).

Change-Id: If3d4dfbed494e7ff9f32514e539cfa089aea18ac

resources/mediawiki/mediawiki.js
tests/qunit/suites/resources/mediawiki/mediawiki.test.js

index 885fd3d..57f85d8 100644 (file)
@@ -2394,7 +2394,7 @@ var mw = ( function ( $, undefined ) {
                                         * @chainable
                                         */
                                        fire: function () {
-                                               return list.fireWith( null, slice.call( arguments ) );
+                                               return list.fireWith.call( this, null, slice.call( arguments ) );
                                        }
                                };
                        };
index f5091f9..41b0cb7 100644 (file)
 
        } );
 
-       QUnit.test( 'mw.hook', 10, function ( assert ) {
+       QUnit.test( 'mw.hook', 12, function ( assert ) {
                var hook, add, fire, chars, callback;
 
                mw.hook( 'test.hook.unfired' ).add( function () {
                } );
                mw.hook( 'test.hook.data' ).fire( 'example', ['two'] );
 
-               mw.hook( 'test.hook.chainable' ).add( function () {
-                       assert.ok( true, 'Chainable' );
-               } ).fire();
+               hook = mw.hook( 'test.hook.chainable' );
+               assert.strictEqual( hook.add(), hook, 'hook.add is chainable' );
+               assert.strictEqual( hook.remove(), hook, 'hook.remove is chainable' );
+               assert.strictEqual( hook.fire(), hook, 'hook.fire is chainable' );
 
                hook = mw.hook( 'test.hook.detach' );
                add = hook.add;