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 28097 - surroundContents should check host-inclusive
Summary: surroundContents should check host-inclusive
Status: RESOLVED WONTFIX
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: DOM (show other bugs)
Version: unspecified
Hardware: All All
: P2 normal
Target Milestone: ---
Assignee: Anne
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-25 02:28 UTC by Koji Ishii
Modified: 2018-03-22 18:02 UTC (History)
5 users (show)

See Also:


Attachments

Description Koji Ishii 2015-02-25 02:28:50 UTC
Range.surroundContents() method[1] checks "partially-contained"[2] pre-check without using host-inclusive, but then in later step when to append[3], its pre-check is host-inclusive.

This should be consistent.

Propose to change "partially contained" to use "host-including inclusive ancestor".

[1] http://w3c.github.io/dom/#dom-range-surroundcontents
    https://dom.spec.whatwg.org/#dom-range-surroundcontents
[2] http://w3c.github.io/dom/#partially-contained
    https://dom.spec.whatwg.org/#dom-range-surroundcontents
[3] http://w3c.github.io/dom/#concept-node-append
    https://dom.spec.whatwg.org/#concept-node-append
[4] http://w3c.github.io/dom/#partially-contained
    https://dom.spec.whatwg.org/#partially-contained
Comment 1 Anne 2015-02-25 13:40:01 UTC
As far as I can tell you hit the host-including check as part of the insertion operation of step 5.

The problematic scenario appears to be that you have a range that contains a <template> and that <template> contains a node that you use as newParent argument.

That would argue for a new step I think, as step 1 does not care about newParent.


Also, how can newParent ever be a Text, Comment, or ProcessingInstruction node? Is that for when the range is collapsed? But if it's not collapsed it seems those nodes would be problematic...
Comment 2 Koji Ishii 2015-08-17 05:34:00 UTC
> As far as I can tell you hit the host-including check as part of the insertion operation of step 5.

Right, and in step 4, the spec "replace all with null within newParent", so failing in step 5 means that UA deletes all children of newParent.

Shouldn't we fail in step 1 instead?
Comment 3 Koji Ishii 2015-08-17 05:36:47 UTC
Demo: http://jsbin.com/secuxo/edit?html,output
Comment 4 Anne 2016-03-14 16:21:31 UTC
Sorry for getting back to you so late, I'm still a bit at a loss as to what to do here. It seems browsers don't throw for this scenario... I guess they didn't update their implementation...
Comment 5 Anne 2018-03-22 18:02:27 UTC
I think I'm fine with this behaving the way it is. Range mutations are already a little weird as they perform many DOM mutations each of which can have side effects.

If you still think this should be fixed, please file a new issue over at https://github.com/whatwg/dom/issues/new.