Because the author has to have the node in question in hand, it seems like document.contains returning an accurate value for elements in Shadow DOM violates the letter of encapsulation but not the spirit.

I think document.contains should return a value reflecting whether the argument is in the document (including shadow trees.)

(In reply to comment #1)
> Because the author has to have the node in question in hand, it seems like
> document.contains returning an accurate value for elements in Shadow DOM
> violates the letter of encapsulation but not the spirit.
> 
> I think document.contains should return a value reflecting whether the
> argument is in the document (including shadow trees.)

That's a neat way to look at the problem. I am convinced.

I think this is the wrong solution. `contains` has always meant that I can walk from the node's parentNode to the ancestor. This changes the semantics of contains.

What is the scenario we are trying to solve? Figuring that out, we can then come up with a reasonable API without changing the semantics of existing methods.

Let me flip the coin a bit. I've been using contains as a signal that the node is not in the main document (in a shadow tree or a disconnected tree).

I ran across this issue this week, and Daniel pointed me at this bug. See the thread at https://groups.google.com/forum/#!topic/polymer-dev/mGs_zkA_7-4.

Erik was asking for a scenario, and this thread presents one. An overlay (popup) element wants to track and/or absorb all clicks on the document. Clicks outside the overlay should dismiss the overlay, while clicks inside the element should have no effect. The overlay author wires up a click handler on the document, capturing events so that it gets first crack at them (and stop them before they might inadvertently triggering other handlers). The overlay's click handler looks at the event target to decide whether the target is in the overlay or not. For this purpose, the developer decides to use contains(). The dev tests the overlay's click tracking behavior with light DOM content inside the overlay, and all works well.

Later, someone else (me) tries to include the overlay element in the shadow of another element, and distribute content into the overlay. When the user looks at the overlay, they see content presented inside the overlay. From the user's point of view, the overlay contains the content. However, if the user clicks on the (light DOM) content, the overlay's contains() check fails, and the overlay fails to behave properly.

One could argue the overlay element should be written differently. Maybe it could track clicks outside using capturing and track clicks inside using bubbling, or find some other solution. But I think the dev made a reasonable call to use contains() for this purpose, and it seemed to work for any tests they cared to make. It was only when the overlay element was subsumed into the shadow of another element that things broke.

I'll hazard that similar issues are likely to come up wherever a dev tries to use contains() within a custom element. Subsuming that element into the shadow of another element will cause problems. I believe custom elements will have a general need to know whether they (visually) contain a given element, regardless of whether the element is in their shadow or distributed to them.

How this is done, whether through modifying contains() or something new, I'll leave to people wiser than me to figure out.

I think we can simulate the proposed *contains()* in JavaScript by using InsertionPoint.getDistributedNodes() and Node.getDestinationInsertionPoints().
I am aware that this will be burden for users, but it could be possible though I've not tried.

I don't think it's good idea to change the semantics of existing contains(). It should consider only a node tree and should ignore the status of the composed tree.

I am not against adding something new in native Shadow DOM if we need it absolutely.

Unfortunately, I think this is a very bad user experience decision.

Yes, we would be breaking the written semantics of document.contains, but we would be keeping the developer expectation that document.contains(node) means "this node is *in* the document".

Otherwise we've broken the developer assumptions about this method, and introduced a whole new variety of bugs.

For example, the jQuery flot charting library draws a helpful chart hover target that follows the mouse. It uses document.contains to determine if it should add page scroll offsets to the mouse x/y position to draw the the target correctly.

In the Polymer ShadowDOM polyfill, we followed the spec, and it broke this library: https://github.com/Polymer/polymer/issues/162.

Ember is another library that uses document.contains to determine if an element should instantiate its controller: https://github.com/Polymer/ShadowDOM/issues/365

Even if we made a new function that does what we want, like Node.prototype.isInDocumentForReals, we still have to knock on all the library authors doors to get them to use the new method, or they say "ShadowDOM broke my stuff, it sucks" and everybody loses.

Web developers need their tools to work, and making document.contains continue to follow developer expectations seems like winning all around.

WDYT?

To interpret the current spec literally, I think:

- document.contains should always return *false* if the argument node is in a shadow tree, as Erik said in comment #3. There is no ancestor/descendant relationship if they are in the different node trees.

I understand that this might cause a very bad user experience.

How do we proceed? Let me propose some ideas.

A) Change the semantics of node.contains so that it should consider the tree of trees.
   That means we are extending ancestor/descendant relationships beyond one node tree. I am afraid that we have to do *monkey-patch* to `node.contains` in the Shadow DOM spec.

B) Leave node.contains as is. That means if the context object and argument node are in the different node trees, it always returns *false*.

 As alternative, we should provide a new API, such as node.inComposedTree(). node.inComposedTree return true if and only if:
   The node participates in the composed tree whose root node is document.

C) Better ideas are welcome.

FYI.

In blink, looks like the semantics of `node.contains` changed in
https://chromiumcodereview.appspot.com/21123005

However, we don't have any spec for that. I'm not convinced with decision.

I prefer Idea B.  WDYT?

If we go for idea B, we need API to satisfy the original requirement.

Because node.inComposedTree() is just an tentative idea, I appreciate opinions for better API to satisfy the requirement.

Other ideas:
- document.containsInComposedTree(node) - I think this is preferred.

Any ideas are welcome!

(In reply to Hayato Ito from comment #9)
> If we go for idea B, we need API to satisfy the original requirement.
> 
> Because node.inComposedTree() is just an tentative idea, I appreciate
> opinions for better API to satisfy the requirement.
> 
> Other ideas:
> - document.containsInComposedTree(node) - I think this is preferred.
> 
> Any ideas are welcome!

I think there's two separate questions here, Document.contains and Node.contains seem like totally separate questions.

"Am I in this Document"
"Am I a child of this Node"

Now that we have ShadowRoot.host you can actually walk up to the root and get the answer you want though, so perhaps reverting this is okay. We should probably add an inDocument() method though.

FYI,
In blink, I've reverted the change of "node.contains": https://codereview.chromium.org/197283016/

(In reply to Elliott Sprehn from comment #10)
> (In reply to Hayato Ito from comment #9)
> > If we go for idea B, we need API to satisfy the original requirement.
> > 
> > Because node.inComposedTree() is just an tentative idea, I appreciate
> > opinions for better API to satisfy the requirement.
> > 
> > Other ideas:
> > - document.containsInComposedTree(node) - I think this is preferred.
> > 
> > Any ideas are welcome!
> 
> I think there's two separate questions here, Document.contains and
> Node.contains seem like totally separate questions.
> 
> "Am I in this Document"
> "Am I a child of this Node"

Agreed. IMO, we should avoid such a double meaning.
We should have a more explicit API instead of re-using 'document.contains'.

> 
> Now that we have ShadowRoot.host you can actually walk up to the root and
> get the answer you want though, so perhaps reverting this is okay. We should
> probably add an inDocument() method though.

Yeah, we should discuss further to pursuit a good API.
I'd like to hear opinions from developers to address their use cases.

Because we already have /deep/ combinator, http://drafts.csswg.org/css-scoping/#selectordef-deep, I think `document.deepContains(node)` might be a good name.

WDYT? If it's okay, let me spec that.

It seems that Document doesn't have `contains`.
http://dom.spec.whatwg.org/#interface-document

We should add `deepContains` to Node, instead of Document.
http://dom.spec.whatwg.org/#dom-node-contains

I've added 'Node.deepContains(Node? other)' in
https://github.com/w3c/webcomponents/commit/14322e526d53cde0814bec041d7fab6dd77f7e88

I need a feedback.
Could someone verify whether this definition satisfies our requirements or not?

I don't really like deepContains (partly because I think CSS is wrong by exposing
shadow dom in that way, and also because 'deep' doesn't really say anything.)

Couldn't we change contains() to take second param, which could be
some dictionary.

bool contains(Node? other, optional ContainsFilter filter)

dictionary ContainsFilter
{
  bool includeShadowDOM = false; // includeShadowDOM is a bit bad name.
}

Thank you for the feedback!

(In reply to Olli Pettay from comment #15)
> I don't really like deepContains (partly because I think CSS is wrong by
> exposing
> shadow dom in that way, and also because 'deep' doesn't really say anything.)
> 
> Couldn't we change contains() to take second param, which could be
> some dictionary.
> 
> bool contains(Node? other, optional ContainsFilter filter)
> 
> dictionary ContainsFilter
> {
>   bool includeShadowDOM = false; // includeShadowDOM is a bit bad name.
> }

It looks better to me than deepContains. I like it.
If no one have better idea than this, I'l change the spec.
In this case, should I make a patch for DOM spec?

Well, includeShadow or some such could work. Or includeShadowNodes. Let's not use the term "DOM" in any API. Ideally, the terms we use for shadow nodes are consistent in some way.

Well, actually, the Shadow DOM spec doesn't define any technical definition of 'Shadow DOM'. That isn't well defined term. We should avoid using the term of 'Shadow DOM' in APIs.

includeNodesInShadowTrees might be the most unambiguous, however it looks too redundant.

My preferences are:

- Most preferred -

A. includeShadow
B. includeShadowTrees
C. includeNodesInShadowTrees
D. includeShadowNodes

- Least preferred -

I don't have a strong opinion either. I am aware this might be bike-shed discussion, but the naming of API is important.

includeShadow seems fine. Shortest and to the point. So I guess we'll have shadow trees and the one <textarea> and friends use are isolated shadow trees? (And won't be exposed here of course.)

Good question.

IMO, document.contains(nodeInShadowTreesHostedByTextAreaElementInDocument, { includeShadow: treu}) should return `true`. However, I'm not confident.

I am not sure whether there is a use case for that, assuming external developers can't get an access for such a node usually.

AFAIK, only use cases I've heard so far is for nodes which are in user-created shadow trees, rather than nodes in UA shadow trees for built-in elements.

If I misunderstand your question, please correct me.

For what's it worth, I managed to solve my need for a document.contains that cuts across shadow boundaries by doing: 

element.matches(':host-context(body) *')

(essentially, I needed to know if a shadow-nested element was in the main document)

+1 to a more versatile document.contains though. I'm not sure what other possible options a dictionary would be good for, so I think just a simple true/false as the second parameter would work well. If there is need for further config in the future, then I guess you can still keep the true/false parameter and overload it with a dictionary.

Moved to https://github.com/w3c/webcomponents/issues/81