jenkins-bot has submitted this change and it was merged.
Change subject: mw.Map: Check presence of value argument in set()
......................................................................
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
---
M resources/src/mediawiki/mediawiki.js
M tests/qunit/suites/resources/mediawiki/mediawiki.test.js
2 files changed, 7 insertions(+), 5 deletions(-)
Approvals:
Fomafix: Looks good to me, but someone else must approve
Jforrester: Looks good to me, approved
jenkins-bot: Verified
diff --git a/resources/src/mediawiki/mediawiki.js
b/resources/src/mediawiki/mediawiki.js
index ff7012f..e0e2963 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 6c8c62f..c948274 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
--
To view, visit https://gerrit.wikimedia.org/r/184560
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4c69f0c11f165640a9b387a72c77c48eb6aa9e72
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
Gerrit-Reviewer: Fomafix
Gerrit-Reviewer: Jack Phoenix <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
