This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 20127 - [Templates]: Template needs to disallow DOM hierarchy cycles which can result from template.contents
Summary: [Templates]: Template needs to disallow DOM hierarchy cycles which can result...
Status: RESOLVED FIXED
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: HISTORICAL - Component Model (show other bugs)
Version: unspecified
Hardware: PC All
: P2 normal
Target Milestone: ---
Assignee: Dimitri Glazkov
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 15476
  Show dependency treegraph
 
Reported: 2012-11-28 18:58 UTC by Rafael Weinstein
Modified: 2013-02-08 17:53 UTC (History)
7 users (show)

See Also:


Attachments

Description Rafael Weinstein 2012-11-28 18:58:05 UTC
e.g.

template.content.appendChild(template).

Note that ShadowRoot has an nearly identical problem, we hopefully we can address both with the same solution.

I propose the right behavior is DOM operation that would result in a cycle via template.content throw a DOM HIERARCHY exception the same way that it does now with parent/child cycles.
Comment 1 Rafael Weinstein 2013-01-24 17:54:24 UTC
This has been implemented both for template element and also for ShadowRoot in WebKit.

Note that I've made a non-normative comment about this in the template element spec. I suspect that maybe DOM needs to say something about this, but I'm not sure how to approach this.

Anne, what's your feeling?
Comment 2 Rafael Weinstein 2013-01-24 17:55:26 UTC
Note that last statement under the template element section:

https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/templates/index.html#template-element
Comment 3 Anne 2013-01-25 13:58:18 UTC
Lets keep this open until it's fixed then okay?

So it seems like in addition to checking the inclusive ancestors as is already done by the pre-insert and replace algorithms, we also need to check the inclusive ancestors of the object the tree is associated with.

Should we maybe introduce a new node that inherits from DocumentFragment that can carry that additional semantic as well as provide a pointer to the hosting element?
Comment 4 Adam Klein 2013-01-25 20:02:29 UTC
(In reply to comment #3)
> Should we maybe introduce a new node that inherits from DocumentFragment
> that can carry that additional semantic as well as provide a pointer to the
> hosting element?

When you say "provide a pointer" do you mean as part of its interface? The WebKit implementation of course does exactly what you describe (a DocumentFragment subclass, a pointer to the host <template>) but doesn't change at the WebIDL level.
Comment 5 Anne 2013-01-26 08:42:57 UTC
Well the idea was a new interface as the functionality might be useful. But I guess for now I can add a concept to DocumentFragment named "host" and then add the appropriate checks to pre-insert and replace in case there's a DocumentFragment root which has a host defined. Would that work?
Comment 6 Rafael Weinstein 2013-01-28 20:47:59 UTC
@Anne, I like your solution in comment 5.

It seems like we should avoid exposing |host| as an IDL property until we have a clear use case.
Comment 7 Anne 2013-01-29 12:36:42 UTC
If someone appends <template>.contents (or whatever that's called), that should just work right?
Comment 8 Adam Klein 2013-01-30 20:50:30 UTC
(In reply to comment #7)
> If someone appends <template>.contents (or whatever that's called), that
> should just work right?

Yes, that'll simply remove everything from the content fragment and append it to wherever it got appended, as it works for any other fragment. But there's no way to "detach" the fragment from the <template> (its "host" property can't change).
Comment 9 Anne 2013-02-06 17:40:47 UTC
https://github.com/whatwg/dom/commit/bc62c366536ace9fde6bbc9f0098d0f17a64aef1
http://dom.spec.whatwg.org/#concept-documentfragment-host

You can now use this host concept in the <template> specification.
Comment 10 Rafael Weinstein 2013-02-06 22:26:24 UTC
@Anne: The language in pre-insert doesn't look sufficient to me. I think you need to recursively traverse host boundaries.

e.g.

<div>
  <template>
    <div>
      <template>

and append the outer div to the inner-most template.content.
Comment 12 Rafael Weinstein 2013-02-07 21:34:26 UTC
Ok. The spec now is clear that when the content document fragment is initialized, its host element is set to the template.

https://dvcs.w3.org/hg/webcomponents/rev/1671f8bac2df

I'm going to go ahead and close this and leave it to Anne to get the language correct in DOM about preventing hierarchy cycles.
Comment 13 Anne 2013-02-08 10:09:41 UTC
Are you saying I did not fix it? I'm confused.
Comment 14 Rafael Weinstein 2013-02-08 17:53:53 UTC
My bad. I didn't noticed that you had fixed it. Looks right to me now. Thanks!