DOMTokenList causes errors with copy() and forEach()

[herrevilkitten]

Assume that there is an object and one of its properties is an object with DOMTokenList as its prototype. In Chrome and FireFox, this will cause different errors when copying the object. In both cases, the error actually occurs in forEach().

For Chrome:
Chrome will throw an exception at line 267 (of Angular.js) because its version of DOMTokenList implements its own version of forEach, which causes an invalid invocation error.

For FireFox:
FireFox will throw an exception at line 203 (of Angular.js) because of the following:

(in copy)

    var emptyObject = Object.create(Object.getPrototypeOf(source));
    destination = copy(source, emptyObject, stackSource, stackDest);

...

    forEach(destination, function(value, key) {
      delete destination[key];
    });

...

(in isArrayLike)

var length = obj.length;

Essentially, the problem is this:
DOMTokenList has a read-only length property. As an interface, this requires that its implementing object define a length property. A new, empty object is created. It is then iterated over in order to delete any existing properties. During this iteration, forEach() calls isArrayLike(), which subsequently checks to see if there is a length property. This causes an error, as the read-only property is checked and does not exist on the object.

The FireFox issue may also be causing the Chrome issue. I have not fully tested this.

My current workaround is to comment out the part where it calls the native forEach and to change the property removal to just be a for loop.

[herrevilkitten]

I've verified that both the Chrome issue and the FireFox issue can be traced back to the property removal. Replacing the forEach with

                for (key in destination) {
                    if (destination.hasOwnProperty(key)) {
                        delete destination[key];
                    }
                }

fixes it in both.

[herrevilkitten]

This is our code that is causing the problem:

                //Watch for changes to this elements classes
                scope.$watch(function () {
                    return elem.context.classList;
                }, function (classList) {
                    disabled = classList.contains('disabled');
                }, true);

It's part of some click detection code, so we can disable links if they have the disabled class.

[pkozlowski-opensource]

@herrevilkitten putting aside problem with angular.copy not working for DOM objects (which I'm not sure it should as there is no real use-case for this in the Angular itself...) observing DOM mutations in the $watch function (to be clear, I'm refereeing to return elem.context.classList;) is a very, very bad idea from the performance point of view.... Without even asking why you are doing this (AngularJS is all about observing model not DOM...), just be aware that you are slowing down your application considerably (factor of 10 or more?) by going down that path.

So I think there are 2 problems in this issue:

• you are clearly miss-using AngularJS here
• as a consequence of this miss-use you need to copy DOM nodes which we don't support

[herrevilkitten]

Yes. I'm looking at ways to make this more appropriate. Thanks for looking at this.

[pkozlowski-opensource]

@herrevilkitten yes, OK, glad that you've decided to refactor things on your end. As I've said before, I don't think we want to support DOM nodes coping as part of AngularJS core.

[sescobb27]

I had the same issue Error: length getter called on an object that does not implement interface Storage.isArrayLike

the way I fix it was just replacing angular.copy(obj) with angular.extend({}, obj) and that made the work

[caitp]

honestly that just sounds like your typescript .d.ts file needs work

[michaschwab]

Thanks a lot herrevilkitten. Replacing that forEach fixed my problems and got my application to work again. Saved my day. I hope angular will put that into their source code.