webcomponents / polyfills

Web Components Polyfills
BSD 3-Clause "New" or "Revised" License
1.15k stars 166 forks source link

[ShadyDOM] A slot shouldn't report its default content as assignedNodes #70

Open JanMiksovsky opened 7 years ago

JanMiksovsky commented 7 years ago

See http://jsbin.com/cusozof/edit?html,console.

A slot can have direct children that the slot can render as default content if no nodes are assigned to the slot:

<template>
  <slot>
    <div>I'm default content.</div>
  </slot>
</template>

Here, the div is default content. If a component uses the above template as a shadow tree, and if an instance of that component has no children, this default content should render.

In that case, the default content should not be returned in the assignedNodes for the slot. This is per the spec, at least as I read https://dom.spec.whatwg.org/#assigning-slotables-and-slots, which makes no mention of light DOM children. This is how native Shadow DOM behaves in both Chrome and Safari. The polyfill appears to incorrectly report default content as assigned nodes.

This issue is breaking one of the Elix components.

sorvell commented 7 years ago

This just looks like a shortcut/oversight that we should be able to easily resolve.

JanMiksovsky commented 7 years ago

I'm still seeing this in v1.0.20, so I'm assuming this hasn't been fixed yet.

dartess commented 6 years ago

In that case, the default content should not be returned in the assignedNodes for the slot, if the {flatten: true} parameter was not specified.

tested on 1.0.22/webcomponents-sd-ce.js

tests: https://codepen.io/anon/pen/EoXMJE?editors=1010 Chrome: passed Firefox: failed

TimvdLippe commented 6 years ago

I tried to tackle this issue, however other tests started failing and I have no clue why. The tests that fail concern the dynamic removal/addition of slots. They rely on ShadyDOM.nativeTree.textContent(el) which expects the fallbackContent to be there. However, per the specification I understand that this should not be the case.

Therefore my suspicion is that these tests unintentionally rely on the broken behavior. However, I am not certain about this and am unable to make the proper modifications. @sorvell could you please let me know what the following tests are meant to test and why they rely on textContent(el)?

append/remove multiple slots with same name (catchall) to element
composed textContent incorrect: expected 'x-disthi' to equal 'x-disthifallback1fallback2fallback3'
  Context.<anonymous> at shady-content.html:746

append/remove multiple slots with same name to element
composed textContent incorrect: expected 'x-disthi' to equal 'x-disthifallback1fallback2fallback3'
  Context.<anonymous> at shady-content.html:801

change slot name duplicating existing slot names
expected 'fooChild' to equal 'fooChildfoo2'
  Context.<anonymous> at shady-content.html:845

composition around slots
expected 'a' to equal 'foo1foo2a'
  Context.<anonymous> at shady-content.html:888
TimvdLippe commented 6 years ago

On this branch you can run the test for this issue: https://github.com/webcomponents/shadydom/tree/assignednodes-fallback

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.