aurelia / aurelia/templating

Internet Explorer: ViewFactory.create does not clone child template content

Open
#569 5 comments 0 reactions 0 assignees View on GitHub
bug has-pr help wanted
Dominant language
TypeScript
Stars
113
Forks
101
PR merge metrics
No merged PRs in 30d

Description

The `ViewFactory` class is responsible for creating new `View` instances. Each time the view factory creates a view it uses the [cloneNode](https://developer.mozilla.org/en-US/docs/Web/API/Node/cloneNode) method to make a deep clone of a component's html template. The clone is what is ultimately data-bound to the view model instance and attached to the DOM.

In Internet Explorer there's an issue with cloning templates that contain other `` elements. Internet Explorer doesn't have true `` element support, which means it's `cloneNode` logic doesn't clone the [`content: DocumentFragment`](https://developer.mozilla.org/en-US/docs/Web/API/HTMLTemplateElement/content) property.

Here's a [plunker](https://plnkr.co/edit/JbnJOGh7Ge2baFYB2elG?p=preview) that demonstrates how this can cause a stack overflow in Internet Explorer. This issue was originally reported in #460.

One way we could fix this issue is to update ViewFactory.create's cloneNode logic. Here's what that might look like:

[plunker](https://plnkr.co/edit/JCAByeyLqzwtyxgL1A4I?p=preview)

```js
import {ViewFactory} from 'aurelia-templating';
import {FEATURE} from 'aurelia-pal';

ViewFactory.prototype.standardCreate = ViewFactory.prototype.create;
ViewFactory.prototype.create = function(container, createInstruction, element) {
if (!FEATURE.htmlTemplateElement && !this.template.__safeToCloneNode) {
const templates = this.template.querySelectorAll('template');
if (templates.length === 0) {
this.template.__safeToCloneNode = true;
} else {
this.template.standardCloneNode = this.template.cloneNode;
this.template.cloneNode = function(deep) {
const clone = this.standardCloneNode(deep);
if (deep) {
const clonedTemplates = clone.querySelectorAll('template');
let i = clonedTemplates.length;
while (i--) {
clonedTemplates.item(i).content = templates.item(i).content;
}
}
return clone;
};
this.template.__safeToCloneNode = true;
}
}

return this.standardCreate(container, createInstruction, element);
};
```

This adds logic to check whether we have true `` element support and lazily polyfilling content property cloning support in individual template instances, on an as-needed basis. When the template does not have any child elements, no polyfilling is needed.

@EisenbergEffect could you review this when you get a chance? Several developers have successfully patched their projects with this fix. If this approach looks good I'll continue working on landing this fix or helping someone else take it the rest of the way. This is a good one for learning the templating internals.

Contributor guide

Open the contributing guide

Research direction

Start at the ViewFactory.create entry point and review the cloneNode behavior described in the issue, using the linked Plunker reproduction in Internet Explorer. Compare the nested template case with a template without child templates, and consider the issue's proposed behavior complete when child template content is preserved without the stack overflow.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, typescript
Domain
frontend, web-dev
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.