jenkins-bot has submitted this change and it was merged.
Change subject: ve.isMixedIn: Fix bugs and add unit tests
......................................................................
ve.isMixedIn: Fix bugs and add unit tests
* It returned undefined instead of false if the subject
didn't have a 'mixins' property. This is because '&&'
is a DEFAULT operator, not an AND operator, it returns the
value, not a boolean per se.
* The logic of traversing to the constructor property was
broken since all objects in javascript have a constructor
property, and functions/constructors are also objects.
Follows-up Ic3e4472b9e694.
Change-Id: I462e7ce270c8cfc7e1970e359894ee4b7d90b881
---
M modules/ve/test/ve.test.js
M modules/ve/ve.js
2 files changed, 30 insertions(+), 4 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve/test/ve.test.js b/modules/ve/test/ve.test.js
index 6ed5a46..2f04beb 100644
--- a/modules/ve/test/ve.test.js
+++ b/modules/ve/test/ve.test.js
@@ -118,6 +118,31 @@
// ve.mixinClass: Tested upstream (K-js)
+QUnit.test( 'isMixedIn', 11, function ( assert ) {
+ function Foo () {}
+ function Bar () {}
+ function Quux () {}
+
+ ve.inheritClass( Quux, Foo );
+ ve.mixinClass( Quux, Bar );
+
+ var b = new Bar(),
+ q = new Quux();
+
+ assert.strictEqual( ve.isMixedIn( Foo, Function ), false, 'Direct
native inheritance is not considered' );
+ assert.strictEqual( ve.isMixedIn( Foo, Object ), false, 'Indirect
native inheritance is not considered' );
+ assert.strictEqual( ve.isMixedIn( Quux, Foo ), false, 've.inheritClass
does not affect mixin status' );
+ assert.strictEqual( ve.isMixedIn( Foo, Foo ), false, 'Foo does not
mixin Foo' );
+ assert.strictEqual( ve.isMixedIn( Bar, Foo ), false, 'Bar does not
mixin Foo' );
+ assert.strictEqual( ve.isMixedIn( Quux, Bar ), true, 'Quux has Bar
mixed in' );
+ assert.strictEqual( ve.isMixedIn( Bar, Quux ), false, 'Bar does not
mixin Quux' );
+
+ assert.strictEqual( ve.isMixedIn( q, Foo ), false, 've.inheritClass
does not affect mixin status' );
+ assert.strictEqual( ve.isMixedIn( b, Foo ), false, 'b does not mixin
Foo' );
+ assert.strictEqual( ve.isMixedIn( q, Bar ), true, 'q has Bar mixed in'
);
+ assert.strictEqual( ve.isMixedIn( b, Quux ), false, 'b does not mixin
Quux' );
+} );
+
// ve.cloneObject: Tested upstream (K-js)
// ve.isPlainObject: Tested upstream (jQuery)
diff --git a/modules/ve/ve.js b/modules/ve/ve.js
index 9a30c7c..74d96f0 100644
--- a/modules/ve/ve.js
+++ b/modules/ve/ve.js
@@ -157,17 +157,18 @@
};
/**
- * Check if a class or object uses a mixin.
+ * Check if a constructor or object contains a certain mixin.
*
* @param {Function|Object} a Class or object to check
* @param {Function} mixin Mixin to check for
- * @return {Boolean} Class or object uses mixin
+ * @return {boolean} Class or object uses mixin
*/
ve.isMixedIn = function ( subject, mixin ) {
- if ( subject.constructor ) {
+ // Traverse from instances to the constructor
+ if ( $.type( subject ) !== 'function' ) {
subject = subject.constructor;
}
- return subject.mixins && subject.mixins.indexOf( mixin ) !== -1;
+ return !!subject.mixins && subject.mixins.indexOf( mixin ) !==
-1;
};
/**
--
To view, visit https://gerrit.wikimedia.org/r/62667
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I462e7ce270c8cfc7e1970e359894ee4b7d90b881
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Trevor Parscal <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
