jquery / jquery

jQuery JavaScript Library
https://jquery.com
MIT License
59.18k stars 20.61k forks source link

Safari 8: jQuery(2.2.3).isPlainObject(localStorage) !== jQuery(2.1.3).isPlainObject(localStorage) #3045

Closed rdehouss closed 8 years ago

rdehouss commented 8 years ago

Hello,

With jQuery 2.1.3, isPlainObject of localStorage for example was giving false while now, with jQuery 2.2.3, it gives true in Safari 8.

jQuery.isPlainObject(localStorage)

I assume it's a regression and not a drop of support of Safari 8 between jQuery 2.1 and jQuery 2.2, is it correct?

jQuery.isPlainObject(localStorage) should always return false.

I couldn't find quickly a bugfix for this.

Cheers,

Raphaël

gibson042 commented 8 years ago

This is fixed on master, but there was reluctance to backport e0d3bfa77073a245ca112736a1ed3db07d5adcf6. I think you'll have to wait for 3.0.

mgol commented 8 years ago

This behavior is present in all jQuery 1.x versions from 1.5 (released 5 years ago) to 1.12 (i.e. the latest one), only some 2.x versions diverged. I'd rather leave it as it is and tell people to upgrade to 3.0 when it comes out. We should only fix critical issues in 1.x/2.x now and a 5-year-old issue is hardly critical.

mgol commented 8 years ago

BTW, this is not just Safari 8, Safari 9 suffers it as well because constructor is an own property of localStorage there. It's fixed in Safari Technology Preview.

timmywil commented 8 years ago

I agree. Since this is fixed in master, we can close.

markelog commented 8 years ago

Should we add a test for it though?

mgol commented 8 years ago

@markelog Not a bad idea. I'd include more built-ins in such a test.

markelog commented 8 years ago

Too late, already committed :), i think you mean host objects? We have couple tests for those, what you have in mind? I can do a follow-up

mgol commented 8 years ago

:) I meant other host objects but I now see there are some tests for them already.

We could add history, indexedDB, maybe crypto.