IE10 issue: views[] array inside renderCollection loses el.innerHTML

[CNovakCAA]

Bare with me, this is not easy or simple to explain.

The array you create called "views" on line 228 which is then used to store "copies" of the rendered views on line 253 is causing problems for Internet Explorer 10.

The problem is, IE10 maintains a reference between the original view and the "copy" of the view stored in views[], so when you set containerEl[0].innerHTML = '' on line 265, IE10 also wipes the innerHTML of the "copy" of the view which was stored in views[]. (All other browsers leave the innerHTML of the view stored in views[] intact.)

Then, when addView() is called for each view in reRender(), say, when a new model is added to the collection via create(), containerEl tries to append the retrieved view (from line 250) to itself on line 259, but the view.el's innerHTML is blank at that point because of line 265.

This causes all views to disappear when a new model is added to a collection and the views are reRender()ed. You can test this behavior in IE10 by doing the following:

function reRender() { // empty without using jQuery's empty (which removes jQuery handlers) for (var i = 0, l = views.length; i < l; i++){ console.log(views[i].el.innerHTML); } containerEl[0].innerHTML = ''; for (var i = 0, l = views.length; i < l; i++){ console.log(views[i].el.innerHTML); } collection.each(addView); }

Looping the views[] array before and after the containerEl clears its HTML gives you a different result in IE10 vs any other browser. In IE10, the views in views[] all have correct innerHTML before containerEl clears its own innerHTML, and then empty innerHTML after. In all other browsers, the innerHTML in the views[] array is correct both before and after.

I tried several solutions to this bug, but have not solved it. I attempted to put a bandaid on the problem with my pull request #19, but that caused unexpected problems with event bindings. One solution was to pass the view through JSON.stringify then JSON.parse to create a true "copy" of the view to store in the views[] array, but while IE10 was OK with that, the other browsers choked while trying to stringify a circular ref inside the view object. Other attempts to create true copies of the view object to store in the views[] array also failed.

Somehow, the copy of the view stored in views[] needs to be "detached" from the original view object, otherwise IE10 will always destroy the innerHTML of all the views that get passed through reRender(), leaving blank markup on the page.

A fix for this would remove the need for PR #19, which is now clearly not the correct solution.

[HenrikJoreteg]

@CNovakCAA can you show me the code in your app where you use renderCollection and describe the issue you're having?

[lukekarrys]

I'd like to get to the root of the problem, but as another bandaid, I'm curious if something like lodash's clonedeep would work? I believe it handles circular references.

[CNovakCAA]

Line 197 in ./clientapp/pages/options.js in phonesheet5 development branch calls collection.create(), which in turn calls reRender(), which then calls addView().

The result should be a new call status being generated then added to the collection and rendered in the view, but in IE10, all statuses disappear as their view.el.innerHTML is set to an empty string in reRender().

[HenrikJoreteg]

So in IE10 you still have a root element for each call status in the container element, right? But it's empty, is that what you're saying?

[HenrikJoreteg]

the tests all pass on IE10: https://cloudup.com/cjD9YGronzk

Going to try to write a test for the specific problem you're having, though.

[HenrikJoreteg]

That was a weird one.

Was able to reproduce what you described in the test suite and then make a minor syntax change that fixed it.

Apparently in IE10 when you've got a parent element with children and set innerHTML = ""; on the parent to clear it. It appears that some IE engineer decided that as a developer when I did that, it also means that I want to set .innerHTML of all the child nodes to empty string as well, rather than just de-referencing them.

Anyway, when I loop through the views and remove them manually it works.

[gdibble]

:+1:

[CNovakCAA]

@HenrikJoreteg, I actually think the IE10 behavior is the correct one. The view that was being stored in the views[] array was an object, passed by reference, into any array (essentially another variable). That means changes to the original object would affect the referenced one in the array, which is the correct implementation. I think this is less about children of a DOM element being affected by innerHTML = "", and more about how JavaScript passes objects by ref, not value.

IMO, the other browsers got it wrong, IE10 was doing the right thing, surprisingly.

[HenrikJoreteg]

Yes, the views are passed by reference, as all objects are.

But i'm setting .innerHTML of the parent container, why would that have anything to do with the elements within it at all?

There is no such thing as a copy or clone going on anywhere here. Each view in views array is a single object in memory. It doesn't matter if i put that same view into 50 different arrays, it's not being copied ever. So I don't follow your logic.

[CNovakCAA]

I agree, there is no copy / clone going on. I thought looping addView() retrieved the view from the views[] array, and you were expecting to retrieve the innerHTML from that. But, IE10 cleared out the view.el.innerHTML in the array when the innerHTML was cleared from the original view object.

You're saying containerEl inside the renderCollection() fn is the container for the view of the collection (top level view)? And when you pass {containerEl: container} into view.render() (line 256), that ref is telling the sub-view that "container" is its parent view's element?

In that case, no, IE10 shouldn't empty the sub-views' innerHTML when you clear the parent view's innerHTML. Seems like jQuery ran into this same bug 2 years ago:

http://bugs.jquery.com/ticket/11473

Microsoft's answer to jQuery was "this won't be fixed in IE10 because it's too much work."

[HenrikJoreteg]

Ah, right, the variable naming was confusing. Yeah, i'm talking about the parent container that holds all the rendered views for the collection.

Anyway, seems all is well.

published latest as 1.6.5