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

diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js
index ff7012f..e0e2963 100644 (file)
--- 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 6c8c62f..c948274 100644 (file)
--- 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