Skip to main content

Bug Tracker

Side navigation

#13571 closed bug (wontfix)

Opened March 06, 2013 08:01AM UTC

Closed March 15, 2013 12:46AM UTC

Last modified August 06, 2013 10:32PM UTC

"isPlainObject" losts compatibility against 1.9.1

Reported by: ystk_skm (brilliantpenguin@gmail.com) Owned by: rwaldron
Priority: undecided Milestone: None
Component: unfiled Version: 2.0b2
Keywords: Cc:
Blocked by: Blocking:
Description

You are modifying "isPlainObject" method and it is now losts compatibility against 1.9.1.

When some prototype is set via Object, the prototype have "isPrototypeOf" method.

Please check an example for the matter:

http://localhost/cloudplus.me/EasyCoding/

# my checking conditon:

Google Chrome 25.0.1364.152 (Official Build 185281)

OS Mac OS X

WebKit 537.22 (@144108)

JavaScript V8 3.15.11.16

Attachments (0)
Change History (17)

Changed March 06, 2013 08:14AM UTC by ystk_skm (brilliantpenguin@gmail.com) comment:1

I'm sorry for my mistake.

The correct url is: http://cloudplus.me/EasyCoding/

Changed March 06, 2013 01:34PM UTC by ajpiano comment:2

#13572 is a duplicate of this ticket.

Changed March 06, 2013 02:50PM UTC by rwaldron comment:3

owner: → rwaldron
status: newassigned

Changed March 06, 2013 07:22PM UTC by rwaldron comment:4

Confirm that this is a "regression", but jQuery 1.9.x was wrong.

The constructed object _IS_ a plain object, because the code is paving over the constructor's prototype:

function C() {}
C.prototype = {};
c = new C();
console.log(c.constructor);
// > function Object() { [native code] }

If the program resets the constructor, the expected results:

function C() {}
C.prototype = {
  constructor: C
};
c = new C();
console.log(c.constructor);
// > function C() {}

In jQuery.isPlainObject's case, the OPs example code only works in jQuery 1.9.x because it has properties on the object that was used as the prototype, which is what jQuery 1.9.x used to determine property ownership and therefore detect possible inheritance chains.

Everything works the way you want it to until you try to test this:

var c, extended;
function C() {}
C.prototype = {};
c = new C();

extended = jQuery.extend( true, {}, {
  target: c
});

strictEqual( extended.target, c, "Deep extended objects retain identity" );

... Without any properties on that prototype, the jQuery 1.9 way will fail as well: http://jsfiddle.net/rwaldron/7Jg8Q/

Unfortunately, the regression stands and I'll have to restore the behaviour, but it will still be broken (ie. it's correct, but the expectations of the calling code is wrong).

Changed March 06, 2013 07:42PM UTC by Rick Waldron comment:5

resolution: → fixed
status: assignedclosed

Fixes #13571. jQuery.isPlainObject 1.9.x compatibility

Signed-off-by: Rick Waldron <waldron.rick@gmail.com>

Changeset: 8d1c42296fefc4e79189b6c2d78a8a78fdd9576d

Changed March 06, 2013 08:13PM UTC by rwaldron comment:6

resolution: fixed
status: closedreopened

Changed March 06, 2013 08:14PM UTC by rwaldron comment:7

We've decided that this breaking change will remain in 2.x, but will be accompanied by documentation explaining the issue encountered here.

Reverted: https://github.com/jquery/jquery/commit/5c82d36f194a854a19f81bebd3ed1a6e7559c811

Changed March 07, 2013 01:11AM UTC by ystk_skm <brilliantpenguin@gmail.com> comment:8

rwaldron

I can accept your decision easily.

Changed March 07, 2013 03:30AM UTC by rwaldron comment:9

wow, thanks for being so reasonable :) that's refreshing... you just made my night

Changed March 07, 2013 04:14AM UTC by ystk_skm <brilliantpenguin@gmail.com> comment:10

I know that less code is fast code. Thanks for your good decision.

Take care for the same kind of claim for "isPlainObject" not to change again (and you try to revert again).

Changed March 13, 2013 01:33AM UTC by timmywil comment:11

status: reopenedassigned

Changed March 13, 2013 02:49AM UTC by rwaldron comment:12

@timmywil, why is this reopened? (I'm just curious)

Changed March 15, 2013 12:05AM UTC by timmywil comment:13

@rwaldron: Wait, I'm confused now. You reopened it. I just put it back to assigned.

Changed March 15, 2013 12:46AM UTC by rwaldron comment:14

resolution: → wontfix
status: assignedclosed

Yep, you're right. Oops!

Changed April 03, 2013 12:57AM UTC by rwaldron comment:15

#13715 is a duplicate of this ticket.

Changed April 03, 2013 01:31AM UTC by m_gol comment:16

Why the check:

!core_hasOwn.call(obj, "constructor")

is needed in jQuery 1.x anyway? All unit tests are passed by oldIE when this line is missing.

If it's really needed, a unit test for that should be written.

Changed August 06, 2013 10:32PM UTC by rwaldron comment:17

#14227 is a duplicate of this ticket.