From 9674a17c0f3b5ef50ea572bdcbd4b277aa3d956d Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Thu, 3 Apr 2014 18:35:33 -0700 Subject: [PATCH] mw.hook: Make hook.fire actually chainable 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 | 2 +- tests/qunit/suites/resources/mediawiki/mediawiki.test.js | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/resources/mediawiki/mediawiki.js b/resources/mediawiki/mediawiki.js index 885fd3da65..57f85d8516 100644 --- a/resources/mediawiki/mediawiki.js +++ b/resources/mediawiki/mediawiki.js @@ -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 ) ); } }; }; diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js index f5091f967b..41b0cb7035 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js @@ -851,7 +851,7 @@ } ); - 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 () { @@ -869,9 +869,10 @@ } ); 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; -- 2.20.1