jquery / jquery

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

Breaking change to the data module in 3.5.0 #4665

Closed rjaros closed 4 years ago

rjaros commented 4 years ago

The change to the data module made here:

https://github.com/jquery/jquery/pull/4603/files#diff-38fa4ad21a97c2bf8d5b91d033df3335

breaks projects that depend on hasOwnProperty() function availability.

An example: https://github.com/snapappointments/bootstrap-select/issues/2430 Many other project can be affected as well.

Such change should not be introduced as a minor version upgrade.

mgol commented 4 years ago

Thanks for the report. I agree this is an unintended breaking change that we should fix in 3.5.1. Marking for team discussion.

Perhaps we can leave the current solution for jQuery 4.0.0 and in 3.5.1 apply original @gibson042’s suggestion of appending a space to event names before storing in the internal data storage. This solution can break code that relied on the private jQuery._data API to have real event names as event keys without any space at the end but since it’s a private API, this looks more acceptable to me.

Some possible consumer are web browsers, I think Firefox is using this API for jQuery event support in DevTools. If that’s still the case, do you know, @bzbarsky, who at Mozilla we could contact to coordinate any changes to this API?

rjaros commented 4 years ago

Any chance on quick 3.5.1 release?

mgol commented 4 years ago

@rjaros our release process is pretty involved and we still need to discuss & iron out the fix. Don’t expect it within days, more like a few weeks.

mgol commented 4 years ago

We’ll monitor the situation. If we get info about many more projects broken by this change, we’ll try to get the fix out quicker. No promises about specific timing, though: it’s Easter and we’re all volunteers.

XhmikosR commented 4 years ago

@mgol it seems we are affected by the aforementioned change too: https://github.com/twbs/bootstrap/issues/30553

Should I open another issue?

mgol commented 4 years ago

@XhmikosR no, let’s track all similar bugs in this issue.

XhmikosR commented 4 years ago

So, basically the failures can be seen here where I updated jQuery to v3.5.0 and adapted our tests: CI errors

The code is quite old, since our v3.3.4.

the-hotmann commented 4 years ago

Indeed this is also affecting our system (InvoicePlane), jQuery 3.5.0 is tiggering many erros and also breaking the whole JS functonality. Indeed braking changes should not be implemented in a normal release, but instead in a mainrelease (^4.0.0) in jQuery as this will automatically break all projects which are based on jQuery and trust it to not include breaking changes within the same mainversion.

Also we had to change our package.json to not update to jQuery 3.5.0 and stay on ~3.4.1 for not running into this issue.

For us the eror was: jQuery.Deferred exception: e.hasOwnProperty is not a function TypeError: e.hasOwnProperty is not a function

dmethvin commented 4 years ago

We appreciate the quick notification on this breakage and intend to release an update relatively soon but as @mgol said it's Easter weekend. Whatever you were doing on Friday morning before 3.5.0 was released, continue to do that until 3.5.1 is released.

bzbarsky commented 4 years ago

@jasonLaster or @nchevobbe might know whether there are Mozilla dependencies here?

mgol commented 4 years ago

Thanks, Boris. @jasonLaster / @nchevobbe could you point me to the code that handles jQuery listeners in Firefox?

Same question to @paulirish; Chrome supports framework listeners and this could affect that support.

nchevobbe commented 4 years ago

Hello @mgol This is where we handle jQuery events: https://searchfox.org/mozilla-central/rev/8ed108064bf1c83e508208e069a90cffb4045977/devtools/server/actors/inspector/event-collector.js#406-409 . Let us know of any changes so we can take that into account (although it would take a couple months for our changes to hit Firefox release).

mgol commented 4 years ago

I was thinking about it a bit more, my conclusions:

  1. I was focused on all the changes from commit 9d76c0b163675505d1a901e5fe5249a2c55609bc but the change to events should actually be fine. This is because the only way you can get ahold of it is via the private jQuery._data API which is private so if you use it, you're on your own. I'll note it's fine for web browsers to use it as it's just for use in DevTools; if it breaks, no website will be broken, the debugging experience will just be worse until the browser catches up.
  2. If we changed the events internal structure from a prototype-less object with regular event type keys to a regular object with keys with a space appended, it has a potential to break more code than the change to a prototype-less object. Although, from my testing it looks like both approaches work in both Firefox & Chrome.

Because of the above, I'd only revert the data change, deferring it to jQuery 4.0.0. PR incoming.

mgol commented 4 years ago

@XhmikosR Note that this fix won't fix all the other breakages with Bootstrap 3 due to the htmlPrefilter changes that we had to do as a security fix: https://jquery.com/upgrade-guide/3.5/

But at least for them there's a workaround for devs using Bootstrap.

XhmikosR commented 4 years ago

@mgol are you sure our issue can be worked around?

The error that affects us all of our versions since v3.3.4 is because of the changes in #4603 (AFAICT) and Object.create which results in Cannot convert object to primitive value errors.

Here is the relevant WIP PR which fixes the issue https://github.com/twbs/bootstrap/pull/30559/files#diff-6ef28a975ef633bb3b3318b9917df9e5R338

mgol commented 4 years ago

@XhmikosR Not this issue, this issue we'll fix, most likely by reverting the change to src/data/Data.js from 9d76c0b163675505d1a901e5fe5249a2c55609bc.

The PR you linked to also applies updates for the changes to jQuery.htmlPrefilter and those will not be reverted so it's likely Bootstrap v3 code will remain incompatible with jQuery 3.5.0+ by default. The htmlPrefilter issue can be worked around by developers using Bootstrap 3 with jQuery 3.5.0+ via one of the steps outlined in https://jquery.com/upgrade-guide/3.5/ that restore jQuery.htmlPrefilter to it's pre-3.5.0 value.

XhmikosR commented 4 years ago

The PR you linked to also applies updates for the changes to jQuery.htmlPrefilter and those will not be reverted so it's likely Bootstrap v3 code will remain incompatible with jQuery 3.5.0+ by default.

Ah, yeah those changes are only in our tests so people should read the release notes when it comes to their code.

I'm mostly concerned about the Object.create change since there are so many versions out there that are affected by that change, and unfortunately there's great fragmentation in versions people use (mostly 3.x which we no longer support and I'd hate to go back and release a new version).

Thanks for the help!

boris-petrov commented 4 years ago

@XhmikosR - are we to understand that for Bootstrap 3 the only fix needed is the one for Object.create (which the jQuery team will fix) and not for jQuery.htmlPrefilter? That is, the workaround in jQuery's upgrade guide is not needed for Bootstrap 3 (because it affects only Bootstrap tests)?

XhmikosR commented 4 years ago

@boris-petrov yeah, no other code seems to be affected from the other changes AFAICT; our CI passes.

@Johann-s please have another look in case I missed something.

mgol commented 4 years ago

PR: #4666

mgol commented 4 years ago

Whoever is affected by this issue: could you verify #4666 fixes it?

XhmikosR commented 4 years ago

I quickly tested this in my 3.5.0 branch by reverting the core change I made, and it seems to work fine.

Let's wait until someone else also confirms to be safe.

rjaros commented 4 years ago

jQuery built from 3.x-data-object branch seems to fix the issue with Bootstrap Select https://github.com/snapappointments/bootstrap-select/issues/2430

KanagarajanK commented 4 years ago

Bootstrap 3.4.1 which Tooltip.prototype.getOptions which has hasOwnProperty and it is failing with jquery 3.5.0

taulinger commented 4 years ago

jQuery built from 3.x-data-object branch fixes the issue with Bootstrap 3.4.1 Tooltip.prototype.getOptions

Sravan-Nayini commented 4 years ago

Hi @mgol May I know when jQuery 3.5.1 will be released ? High Level plan as of now . Actually Our webisite has issues so we are waiting for this #4666 to be merged and released. Thanks.

mgol commented 4 years ago

@Sravan-Nayini I’ve already elaborated on that in https://github.com/jquery/jquery/issues/4665#issuecomment-612422035 & the follow up comment.

paulirish commented 4 years ago

Same question to @paulirish; Chrome supports framework listeners and this could affect that support.

https://source.chromium.org/chromium/chromium/src/+/master:third_party/devtools-frontend/src/front_end/event_listeners/EventListenersUtils.js?q=f:EventListenersUtils.js%20%22function%20jQueryFetcher%22%20-f:out&ss=chromium%2Fchromium%2Fsrc

cc @jackfranklin

jackfranklin commented 4 years ago

It doesn't look like Chrome's DevTools will be affected as we don't use hasOwnProperty in that file - but please ping me if I can help here at all :)

mgol commented 4 years ago

Fixed by #4666.

DennisSuitters commented 4 years ago

Just to let you guys know, Summernote is affected by this as well, and I believe elFinder is as well.

PaulKahl commented 4 years ago

@mgol This issue should not be closed with a simple "Downgrade to 3.4.1" - Many of us are being asked by our companies to perform a mandatory upgrade because of the XSS vulnerability in < 3.5. Further, downgrading doesn't fix the problem in 3.5 - it just avoids it.

This change in 3.5.0 (and 3.5.0.1) affects numerous other libraries, including Bootstrap 4.x. Can we get some sort of ETA on an actual fix?

tacman commented 4 years ago

Dependabot is sending announcements out to every public repo that uses JQuery 3.4 with an automatic pull request to upgrade to 3.5. I expect lots of people will simply accept the PR.

[image: image.png]

On Thu, Apr 30, 2020 at 1:17 PM PaulKahl notifications@github.com wrote:

This issue should not be closed with a simple "Downgrade to 3.4.1" - Many of us are being asked by our companies to perform a mandatory upgrade because of the XSS vulnerability in < 3.5. Further, downgrading doesn't fix the problem in 3.5 - it just avoids it.

This change in 3.5.0 (and 3.5.0.1) affects numerous other libraries, including Bootstrap 4.x. Can we get some sort of ETA on an actual fix?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jquery/jquery/issues/4665#issuecomment-621988809, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXIQJUDYU4CKW22ISKMX3RPGXCRANCNFSM4MF6QFQQ .

mgol commented 4 years ago

It’s customary to close an issue on GitHub when a fix is merged, not when it’s released. The release should happen soon but I cannot provide you an exact date.

PaulKahl commented 4 years ago

I was hoping for an ETA, but I suppose I'll just assume "soon"-ish. (Btw, the "E" in "ETA" doesn't stand for "Exact", but "Estimated".) Can't wait to see the fix in the wild. Thanks!

biow0lf commented 4 years ago

21 days jquery maintainers can't release hot fix for fixing problem with bootstrap. Leaving all bootstrap users with decision "security fix" vs "working bootstrap".

DennisSuitters commented 4 years ago

@biow0lf it's not only Bootstrap that is affected.

0biWanKenobi commented 4 years ago

Guys, please. Pretty please. Release the fix.

mgol commented 4 years ago

jQuery 3.5.1 has been released: https://blog.jquery.com/2020/05/04/jquery-3-5-1-released-fixing-a-regression/

0biWanKenobi commented 4 years ago

It will sound like we're not letting you catch breath, and I hate to be that guy, but.. any ETA on a NuGet update? thanks for the hard work, guys :)

mgol commented 4 years ago

The jQuery team only maintains the jquery npm & Bower packages, the rest is done by the community. You need to ask the Nugget package maintainer to release the updated version.

mryellow commented 4 years ago

I guess this is why linters say to use Object.prototype.hasOwnProperty.call(obj, 'foo')

ManuC84 commented 3 years ago

I'm using these versions and I can't get the toggle "hamburger"(or any other collapse button) feature to work bootrstrap on laravel, any suggestions?

"devDependencies": { "axios": "^0.19", "bootstrap": "^4.5.0", "cross-env": "^7.0", "jquery": "^3.5.1", "laravel-mix": "^5.0.1", "lodash": "^4.17.13", "popper.js": "^1.12", "resolve-url-loader": "^2.3.1", "sass": "^1.20.1", "sass-loader": "^8.0.0", "vue": "^2.5.17", "vue-template-compiler": "^2.6.10" }

ctwillie commented 3 years ago

@ManuC84 Did you run npm install and npm run dev in your laravel project?