Krinkle has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/346491 )
Change subject: util: Simplify isPlainObject and fix two false positives
......................................................................
util: Simplify isPlainObject and fix two false positives
We already covered the following cases properly:
* Objects that inherited from any user class or built-in class
other than Object.prototype. (= not plain)
* Objects that inherited from another plain object. (= not plain)
* Objects created that inherit Object.prototype via 'new Object()'
or literal '{}'. (= plain)
* Objects created without inheritance via Object.create(null)
(= plain)
The following cases were broken, however:
* Objects that inherit an object that has no inheritance.
> var defaults = Object.create(null);
> // assign props to defaults
> options = Object.create(defaults);
This is not plain, but was wrongly detected as plain.
Also fixed in jQuery 3.
* Objects that inherit an object that is plain.
> var defaults = { /* .. */ };
> options = Object.create(defaults);
This is not plain, but was wrongly detected as plain.
Also fixed in jQuery 3.
Also:
* Remove IE8 compat code that is now redundant.
* Upgrade from jQuery 1 to jQuery 3 must be before this
commit as otherwise the new unit tests will fail for
oojs.jquery.js.
Change-Id: Ibc67bbfaa35e88abcc68e09977bb7c390b1fe388
---
M src/util.js
M tests/unit/util.test.js
2 files changed, 25 insertions(+), 25 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/oojs/core refs/changes/91/346491/1
diff --git a/src/util.js b/src/util.js
index 349436f..5fdafb2 100644
--- a/src/util.js
+++ b/src/util.js
@@ -8,25 +8,20 @@
* @return {boolean}
*/
oo.isPlainObject = function ( obj ) {
- // Any object or value whose internal [[Class]] property is not
"[object Object]"
- // Support IE8: Explicitly filter out DOM nodes
- // Support IE8: Explicitly filter out Window object (needs loose
comparison)
- // eslint-disable-next-line eqeqeq
- if ( !obj || toString.call( obj ) !== '[object Object]' || obj.nodeType
|| ( obj != null && obj == obj.window ) ) {
+ var proto;
+
+ // Optimise for common case where internal [[Class]] property is not
"Object"
+ if ( !obj || toString.call( obj ) !== '[object Object]' ) {
return false;
}
- // The try/catch suppresses exceptions thrown when attempting to access
- // the "constructor" property of certain host objects such as Location
- // in Firefox < 20 (https://bugzilla.mozilla.org/814622)
- try {
- if ( obj.constructor &&
- !hasOwn.call( obj.constructor.prototype,
'isPrototypeOf' ) ) {
- return false;
- }
- } catch ( e ) {
- return false;
+ proto = Object.getPrototypeOf( obj );
+
+ // Objects without prototype (e.g., `Object.create( null )`) are
considered plain
+ if ( !proto ) {
+ return true;
}
- return true;
+ // The 'isPrototypeOf' method is set on Object.prototype.
+ return hasOwn.call( proto, 'isPrototypeOf' );
};
diff --git a/tests/unit/util.test.js b/tests/unit/util.test.js
index 3ae382d..0e505ae 100644
--- a/tests/unit/util.test.js
+++ b/tests/unit/util.test.js
@@ -3,19 +3,24 @@
QUnit.module( 'util' );
QUnit.test( 'isPlainObject', function ( assert ) {
+ var obj;
function Thing() {}
// Plain objects
assert.strictEqual( oo.isPlainObject( {} ), true, 'empty plain
object' );
assert.strictEqual( oo.isPlainObject( { a: 1 } ), true,
'non-empty plain object' );
- if ( !Object.create ) {
- // We test Object.create(null) when the environment
supports it for modern browsers
- // However Object.create(null) itself is not an OOjs
feature and if the environment
- // doesn't support it, we skip the test.
- assert.strictEqual( true, true, '[skipped] Objects with
no prototype from Object.create( null )' );
- } else {
- assert.strictEqual( oo.isPlainObject( Object.create(
null ) ), true, 'Objects with no prototype from Object.create( null )' );
- }
+ assert.strictEqual( oo.isPlainObject( Object.create( null ) ),
true, 'empty object with no prototype, via Object.create( null )' );
+ obj = Object.create( null );
+ obj.foo = true;
+ assert.strictEqual( oo.isPlainObject( obj ), true, 'non-empty
object with no prototype' );
+
+ // Non-plain objects (any inheritance other than
Object.prototype is not plain)
+ obj = Object.create( Object.create( null ) );
+ assert.strictEqual( oo.isPlainObject( obj ), false, 'empty
object inheriting from object with no prototype' );
+ obj.foo = true;
+ assert.strictEqual( oo.isPlainObject( obj ), false, 'non-empty
object inheriting from object with no prototype' );
+ obj = Object.create( {} );
+ assert.strictEqual( oo.isPlainObject( obj ), false, 'object
inheriting from plain object' );
// Primitives
assert.strictEqual( oo.isPlainObject( undefined ), false,
'undefined' );
@@ -89,7 +94,7 @@
} catch ( e ) {
threw = true;
}
- assert.strictEqual( threw, false, 'native host object'
);
+ assert.strictEqual( threw, false, 'native Location
object' );
} );
}
--
To view, visit https://gerrit.wikimedia.org/r/346491
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibc67bbfaa35e88abcc68e09977bb7c390b1fe388
Gerrit-PatchSet: 1
Gerrit-Project: oojs/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
