isPlainObject won't work correctly when constructor property is changed

[fhcj2]

When you instantiate a non Object class (not a plain object), and add it its own constructor property, the function will take it for a Plain Object.

See it here: ​http://jsfiddle.net/fhcj2/J8Yrx/

[fhcj2]

I think it's cause of this block of code:

if ( obj.constructor &&
		!core_hasOwn.call( obj.constructor.prototype, "isPrototypeOf" ) ) {
	return false;
}

Since every object has its own prototype, which leads to the constructor who created it, I think it could be changed for:

if (!core_hasOwn.call(Object.getPrototypeOf(obj).constructor.prototype, "isPrototypeOf") ) {
	return false;
}

I tested it and it fixed the problem. What do you guys think? Is it ready for a pull request?

[Rick Waldron]

Duplicate of #13571.

[fhcj2]

Actually this issue is about jQuery 2.0 (had just cloned it from the repository, but forgot to change it here, sry..)

But anyway this is an issue that happens both in 2.0 and 1.9.1 versions of jQuery, and I think it's not related to "Ticket #13571", since it covers a specific situation when the object is added a "constructor" property. Would you explain why you considered it as a duplicate? =\

[m_gol]

Your jsFiddle specificely uses jQuery 1.9.1, not 2.0.x. Also, if you change the version, your test no longer returns true in the last case.

Are you sure you have a problem with specifically jQuery 2.0? Because it doesn't seem so.

[fhcj2]

Just noticed that now that you said... But I even recloned it right now and the error still occurs on the compiled (by grunt) version from the repository. It says in the code:

/*!
 * jQuery JavaScript Library v2.0.0-pre
 * http://jquery.com/
 *
 * Includes Sizzle.js
 * http://sizzlejs.com/
 *
 * Copyright 2005, 2013 jQuery Foundation, Inc. and other contributors
 * Released under the MIT license
 * http://jquery.org/license
 *
 * Date: 2013-04-03
 */

Would that "2.0.0b1" or "2.x" from jsfiddle be any different from this one? Ty for the reply btw =]

[m_gol]

jQuery edge 2.x is code from code.jquery.com/jquery-git2.js which is always the latest jQuery compilation from master. If you think your compiled code does sth different, try comparing those files (using, for example, some command line tool like diff).

Isn't it possible that your browser cached jQuery 1.9? I'd suppose this could be the reason as the jsFiddle clearly shows the problem doesn't exist in jQuery 2.0

[Rick Waldron]

I'm not sure what part is unclear, both are directly related to detecting "plain object"-ness when an object's constructor has been changed—either by defining the prototype with an object (and thereby changing the prototype.constructor property) or by adding an own property that happens to be called "constructor". Once you modify a special property, such as "constructor", all bets are off.

[fhcj2]

Sry, guys.. My mistake. That example would only cover the error in 1.x really. Made another jsfiddle script using the 2.x jQuery version. ​http://jsfiddle.net/fhcj2/cNNA4/ Wouldn't it be an error? The change I proposed would fix it too...

[m_gol]

Your change moves the check one level below. It can fix your specific use case but it's not a golden solution. For one, your code will completely break when invoked on object created via Object.create(null) - null prototype doesn't have a constructor field, obviously.

[fhcj2]

Didn't remember that function.. But I think perhaps a simple "!= null" would solve that problem, right? since it takes an object to reach this code.

var objProto = Object.getPrototypeOf(obj);
if (objProto != null && !core_hasOwn.call(objProto.constructor.prototype, "isPrototypeOf") ) {
	return false;
}

Although it wouldn't cover another (possible)problem which also happens in jQuery 2.x: ​http://jsfiddle.net/fhcj2/jwpqK/

Since an object created this way seems to behave exactly as a "plain object", I'm not sure if it's not right for it to be identified as such. In the case it is not considered a bug, maybe this code would suffice?

[m_gol]

Well, this additional check doesn't help in all cases since you just moved it one level lower. If instead of assigning MyClass.prototype to null you assign it to Object.create(null), the code will throw. Sure, it's catched later but using exceptions for flow control is not a good practice. Not to mention your code no longer recognizes {} as a plain object which kind of misses the point...

Like rwaldron said, when you change the prototype field of the constructor function or the constructor field of an object, all weird kinds of things could happen.

P.S. There is no need to type $(document).ready by hand in jsFiddle, since you chose onLoad in the left jsFiddle pane all the code is invoked after window load event occured (and, thus, jQuery document ready, too).