From d04b7618b566b02cea5e6544c1b4169104da5374 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sun, 11 Jan 2015 06:03:34 -0800 Subject: [PATCH] mw.Map: Check presence of value argument in set() Add back the `arguments.length > 1` check (accidentally removed in 24f84b0). Otherwise it inadvertently uses the `value` parameter, causing it to set undefined as the value. There was already a test ensuring undefined can be set as value, but a test to ensure it doesn't default to undefined was missing. Change-Id: I4c69f0c11f165640a9b387a72c77c48eb6aa9e72 --- resources/src/mediawiki/mediawiki.js | 2 +- .../qunit/suites/resources/mediawiki/mediawiki.test.js | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js index ff7012fab3..e0e2963a68 100644 --- a/resources/src/mediawiki/mediawiki.js +++ b/resources/src/mediawiki/mediawiki.js @@ -191,7 +191,7 @@ } return true; } - if ( typeof selection === 'string' && arguments.length ) { + if ( typeof selection === 'string' && arguments.length > 1 ) { this.values[selection] = value; return true; } diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js index 6c8c62f0d5..c948274d1d 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js @@ -55,7 +55,7 @@ this.restoreWarnings(); } ); - QUnit.test( 'mw.Map', 34, function ( assert ) { + QUnit.test( 'mw.Map', 35, function ( assert ) { var arry, conf, funky, globalConf, nummy, someValues; conf = new mw.Map(); @@ -86,8 +86,10 @@ assert.strictEqual( conf.set( 'constructor', 42 ), true, 'Map.set for key "constructor"' ); assert.strictEqual( conf.get( 'constructor' ), 42, 'Map.get for key "constructor"' ); - assert.strictEqual( conf.set( 'ImUndefined', undefined ), true, 'Map.set allows setting value to `undefined`' ); - assert.equal( conf.get( 'ImUndefined', 'fallback' ), undefined, 'Map.get supports retreiving value of `undefined`' ); + assert.strictEqual( conf.set( 'undef' ), false, 'Map.set requires explicit value (no undefined default)' ); + + assert.strictEqual( conf.set( 'undef', undefined ), true, 'Map.set allows setting value to `undefined`' ); + assert.equal( conf.get( 'undef', 'fallback' ), undefined, 'Map.get supports retreiving value of `undefined`' ); assert.strictEqual( conf.set( funky, 'Funky' ), false, 'Map.set returns boolean false if key was invalid (Function)' ); assert.strictEqual( conf.set( arry, 'Arry' ), false, 'Map.set returns boolean false if key was invalid (Array)' ); @@ -99,7 +101,7 @@ conf.set( String( nummy ), 'I used to be a number' ); assert.strictEqual( conf.exists( 'doesNotExist' ), false, 'Map.exists where property does not exist' ); - assert.strictEqual( conf.exists( 'ImUndefined' ), true, 'Map.exists where value is `undefined`' ); + assert.strictEqual( conf.exists( 'undef' ), true, 'Map.exists where value is `undefined`' ); assert.strictEqual( conf.exists( nummy ), false, 'Map.exists where key is invalid but looks like an existing key' ); // Multiple values at once -- 2.20.1