Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the constraint that slot can't have another slot as an ancestor #321

Closed
rniwa opened this issue Sep 17, 2015 · 16 comments
Closed

Remove the constraint that slot can't have another slot as an ancestor #321

rniwa opened this issue Sep 17, 2015 · 16 comments

Comments

@rniwa
Copy link
Collaborator

rniwa commented Sep 17, 2015

This seems like a completely unwarranted constraint:

There is no other slot element in the ancestors of the slot element

This requirement introduces an inconsistency in the way slots are found in the shadow tree. e.g. s3 instead of s2 is the active slot in the example below despite of the fact s2 appears before s3 in the tree order:

<slot id=s1 name=foo>
  <slot id=s2 name=bar></slot>
</slot>
<slot id=s3 name=bar></slot>

It also prevents a rather compelling use case of nested slots once the fallback contents are supported by slot elements (see the issue #317). e.g.

<slot name=fullName>
  <slot name=firstName></slot> <slot name=lastName></slot>
</slot>
@hayatoito
Copy link
Contributor

Unfortunately, this will make the situation extremely complex.

e.g. How do you define the event path? The sibling nodes of the same shadow host would receive the same event. That would break the assumption of the event dispatching.

Could you take a look at https://w3c.github.io/webcomponents/spec/shadow/#event-paths-example
WDTY?

@hayatoito
Copy link
Contributor

Also remember that a child of the shadow host could have a child node. It's not always a leaf node.

@rniwa
Copy link
Collaborator Author

rniwa commented Sep 17, 2015

There's nothing complicated here. Allowing nested slots would not cause a single event to be dispatched on siblings of a single parent node.

For example, let us examine the following very simple shadow tree:

<slot id=s1 name=foo>
  <span id=s2>
    <slot id=s3 name=bar></slot>
  </span>
</slot>

If an event were to fire at a node assigned to s1, then it'll just propagate up to the shadow root in the shadow tree, and goes to the shadow host. Nothing special about that. If an event fires at a node assigned to s3, it would similarly propagate to s2, s1, and then to s1's parent. None of the nodes assigned to s1 comes into play in that scenario.

@hayatoito
Copy link
Contributor

Yeah, having the fallback behavior is precondition for allowing the nesting. That's my point.

Can we merge this issue to #317 ?

@hayatoito
Copy link
Contributor

I guess one of the motivations is that we don't need to check an ancestor of an slot element to know the given slot is valid one or not, in implementing Shadow DOM, right?

@rniwa
Copy link
Collaborator Author

rniwa commented Sep 17, 2015

Right. The motivation is to avoid the unnecessary validation in engines as well as making the behavior of slots more consistent.

I don't think we should merge the issues. #317 is about enabling fallback contents. This issue is about not having the constraint.

It's true that removing this constraint doesn't allow new use cases without allowing fallback contents for slots, but it still makes the behaviors of slot elements simpler and more consistent.

@hayatoito
Copy link
Contributor

Okay. We might have another option:

  • Remove the constraint, but there is no fallback behavior for slots at least as of now.

In this case, there is no point nesting slots, in terms of the composed tree, however, we remove the constraint anyway. I'm okay to this change as the first step. It doesn't look harmful. WDYT?

@rniwa
Copy link
Collaborator Author

rniwa commented Sep 17, 2015

Yeah, that's totally fine with me as a separate change. We'll see what happens with #317 in terms of fallback contents.

@hayatoito
Copy link
Contributor

I've removed the constraint at: be457d5.

I'm now thinking that I can remove the condition further:

The root node of the slot element is a shadow root

But this requires more investigation. That could be done as another issue.

@rniwa
Copy link
Collaborator Author

rniwa commented Sep 18, 2015

I think requiring the root node to be a shadow root is fine since slot element is really for shadow trees. But like you said, that's a separate issue if anything.

@hayatoito
Copy link
Contributor

After removing, I found that an issue of removing the constraint:

<slot id=s1 name=foo>
  <span id=s2>
    <slot id=s3 name=bar></slot>
  </span>
</slot>

If an event fires at a node assigned to s3, it would similarly propagate to s2, s1, and then to s1's parent. None of the nodes assigned to s1 comes into play in that scenario.

Assuming that both s1 and s3 have a assigned node,
if an event fires at a node assigned to s3, call it bar, the event path would be:

  • The event path: [bar, s3, s2, s1, host, ...]

However, this wouldn't match the ancestor chain of the composed tree. :(

e.g. see s2 and s1 (or s2 and host if we omit slots from the event path). There is no relationship between them in the composed tree.

@hayatoito
Copy link
Contributor

Ops. This issue is already there without removing the constraint.

Suppose the an event fires at s2. :(
Maybe it's okay to have this behavior. It doesn't looks harmful, I hope.

As long as an event fires at a node in a document composed tree, we can live in a clean world.

@hayatoito
Copy link
Contributor

I think requiring the root node to be a shadow root is fine since slot element is really for shadow trees. But like you said, that's a separate issue if anything.

Yeah, in practical, there is no benefits. But no harmful, I guess. Removing the remaining condition would make the spec, and the implementation, simpler and more consistent, I guess.

I will take a look this issue later, after we resolve the fallback issue, that's a necessary condition to remove the remaining condition.

@rniwa
Copy link
Collaborator Author

rniwa commented Sep 18, 2015

Now that I think about it more, I guess that constraint doesn't do anything since slots only get nodes assigned from its shadow host. So regardless of whether we have an explicit constraint or not, slots won't do anything if its root node isn't a shadow root.

@hayatoito
Copy link
Contributor

Yeah, there is no practical benefits. But we have a minor bonus. We can remove the following special behavior (at least we don't have to mention it):

If a slot element does not satisfy the condition of a slot, it must have the same rendering behavior as the HTMLUnknownElement.

If a slot is used in a document tree:

  • No node is assigned to the slot
  • Thus, fallback contents are always used
  • with slot {display: contents}, which is now being proposed.

Thus, we can get a consistency. No special treatment is required for a slot in a document tree.
We can always consider [slot element] == [slot], without any special handling of a slot element in a document tree. That would make me happy.

@rniwa
Copy link
Collaborator Author

rniwa commented Sep 18, 2015

Ah, right. Yes, that would be perfect!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants