Repository navigation
fix(jqLite.cleanData): TypeError: Cannot read property 'events' of undefined #16641
Description
I'm submitting a ...
- [x ] bug report
Current behavior:
TypeError happens when data is cleaned for OBJECT element. OBJECT does not support data
Expected / new behavior:
no TypeError expected
Minimal reproduction of the problem with instructions:
OBJECT element removed by because of ng-if instruction
<div ng-if="shownGraphic.tag === 'multimedia' && shownGraphic.multimediaObject.multimediaType === 'swf'" style="width: 100%; height: 100%;">
<object style="width: 100%; height: 100%;"><embed style="width: 100%; height: 100%;" embed-src="{{shownFigureView.url()}}" /></object>
</div>
AngularJS version:
Angular 1.7.2
JQuery 1.10.2
Browser:
Presumably All
Anything else:
TypeError: Cannot read property 'events' of undefined
at Function.jqLite.cleanData (angular.js:2051)
at replaceWith (angular.js:10704)
at applyDirectivesToNode (angular.js:9768)
at compileNodes (angular.js:9293)
at compileNodes (angular.js:9305)
at compileNodes (angular.js:9305)
at compileNodes (angular.js:9305)
at compile (angular.js:9174)
at Object.link (angular.js:28966)
at angular.js:1364
Activity
gkalpak commentedon Jul 22, 2018
This is a jQuery 1.x issue. Not sure which version it was fixed in, but jQuery 3.x (as well as AngularJS' built-in
jqLite) always return an object.alextkachman commentedon Jul 22, 2018
This is so wrong. It for years worked correctly elem._data('events') and recently was changed to incorrect elem._data().events with no obvious for me reason
gkalpak commentedon Jul 22, 2018
Hm...OK if we can fix this is a non-breaking way, then there is little reason not to, I guess 😃
It was changed in b7d396b.
@mgol, any objection against reverting
jqLite._data(elem).eventstojqLite._data(elem, 'events')? Is this going to affect the behavior with latest jQuery?mgol commentedon Jul 22, 2018
@alextkachman AngularJS hasn't supported jQuery 1.x since version 1.3 if I remember correctly. This version was released almost 4 years ago. Your configuration has been unsupported for a long time so something may randomly break even in a patch release; we can't prevent that from happening as we don't run unit tests with jQuery 1.x. You should move to a supported version of jQuery, i.e. 2.1 or newer.
@gkalpak We can't just revert the change as jqLite doesn't support the two-param
_datasignature and the linked commit changed thecleanDatapatch to be performed on jqLite in addition to jQuery. That's why I changed this line.Fixing this issue requires to do one of the following:
_datato jqLitejQuery._data(elem)being defined before accessing itseventspropertyI think what happens is jQuery 1.x uses
jQuery.cleanDatainsidejQuery#removeinternally where it removes the cache entry for the removed node before callingjQuery.cleanData, makingjQuery._data(elem)returnundefinedinside of our patchedjQuery.cleanData. In all other cases, even jQuery 1.x should be falling back to an empty object if there are no handlers attached.gkalpak commentedon Jul 22, 2018
Thx for the info, @mgol. Although jQuery 1.x is indeed not supported, I think option (3) is harmless (and straight forward). I'll submit a PR.
fix(cleanData): do not break if `jQuery._data()` returns undefined
fix(angular.element): do not break on `cleanData()` if `_data()` retu…
fix(angular.element): do not break on `cleanData()` if `_data()` retu…
mgol commentedon Jul 23, 2018
Yes, I forgot to add option 3 sounded the best to me as well as it's such a critical place and such an easy fix.
alextkachman commentedon Jul 23, 2018
Thank you, @gkalpak and @mgol