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

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

Open
JanMiksovsky opened this issue Nov 3, 2017 · 6 comments

Comments

@JanMiksovsky
Copy link
Contributor

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
Copy link
Collaborator

sorvell commented Nov 15, 2017

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

@JanMiksovsky
Copy link
Contributor Author

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

@dartess
Copy link

dartess commented Jan 2, 2018

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
Copy link
Contributor

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
Copy link
Contributor

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

@dfreedm dfreedm transferred this issue from webcomponents/shadydom Jun 7, 2019
dfreedm added a commit that referenced this issue Jun 11, 2019
@dfreedm dfreedm changed the title A slot shouldn't report its default content as assignedNodes [ShadyDOM] A slot shouldn't report its default content as assignedNodes Jun 12, 2019
@dfreedm dfreedm self-assigned this Aug 14, 2019
@stale
Copy link

stale bot commented Apr 24, 2021

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.

@stale stale bot added the wontfix label Apr 24, 2021
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

6 participants