Belén [:ladybenko] Assignee
	Description • 6 years ago

If we have a custom element with a slot, and the slotted content is text –i.e. isn't wrapped in a tag– it won't be displayed in the markup view inside the shadow DOM tree, and it won't have it's "reveal slot" icon/link.

I'm attaching a screenshot showing how it looks like at the time being.

	Julian Descottes [:jdescottes]
	Comment 1 • 6 years ago

As mentioned on slack, I would like to see a test page for this. The screenshot seems to show that the <slot> is out of the shadow DOM? 

If I understood the scenario correctly, the situation where a text node gets slotted in a shadow DOM <slot> is covered in the last component displayed at https://juliandescottes.github.io/webcomponents-playground/simple/ . Although I don't think we have a unit test for this.

	Julian Descottes [:jdescottes]
	Comment 2 • 6 years ago

Testing with https://belen-albeza.github.io/webcomponents-playground/test-slot-text/

The issue does not occur if the <slot> has > 1 children, which is why it doesn't happen on my page (which has <slot><span>default content</span></slot> => span + textnodes). The reason this is important is that otherwise, we detect that this slot contains only a short text and can be handled via "inlineTextChild". This is the inspector's way of nesting small text nodes inside their parents (so that we can display lines such as <span>short text</span>).

Slots that have slotted content should never consider using inlineTextChild because they will always want to display their slotted content in its own line.

So we should detect this kind of situation and bail out. https://searchfox.org/mozilla-central/rev/5a18fb5aeeec99f1ca1c36a697082c221189a3b9/devtools/server/actors/inspector/walker.js#522-528 seems to be the correct place to do this verification.

	Julian Descottes [:jdescottes]
	Comment 4 • 6 years ago

Comment on attachment 9008070 [details]
Bug 1488769 - Slotted nodes not being displayed in Shadow DOM tree if the node is a text.
r=jdescottes

Julian Descottes [:jdescottes][:julian] has approved the revision.

	Pulsebot
	Comment 5 • 6 years ago

Pushed by balbeza@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a647d0bbf366
Slotted nodes not being displayed in Shadow DOM tree if the node is a text. r=jdescottes

	Raul Gurzau (:RaulG)
	Comment 6 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/a647d0bbf366

	Julian Descottes [:jdescottes]
	Comment 7 • 6 years ago

Timea, could you verify this additional bug? It was not part of the initial target for 63, but it's a fix for a bug we found near the end of 63 cycle, and we would like to uplift it.

STRs are:
- open https://belen-albeza.github.io/webcomponents-playground/test-slot-text/
- Right click on "Slotted text", click "Inspect Element"

ER: you should see a slot, when you expand it you should see a "<#text>" node

AR in 63: you see <slot>Slotted text</slot>

	Julian Descottes [:jdescottes]
	Comment 8 • 6 years ago

Approval Request Comment
[Feature/Bug causing the regression]:1053898 (it's a bug in a new feature that ships only in 63)
[User impact if declined]:inspector shows incorrect data in some situations involving text-nodes and shadow dom
[Is this code covered by automated tests?]:yes in this patch
[Has the fix been verified in Nightly?]:not yet, flagged for verify 
[Needs manual test from QE? If yes, steps to reproduce]: yes STRs in previous comment
[List of other uplifts needed for the feature/fix]:none
[Is the change risky?]:no
[Why is the change risky/not risky?]:straightforward javascript fix
[String changes made/needed]:none

	Pascal Chevrel:pascalc (PTO until August 21)
	Comment 9 • 6 years ago

Comment on attachment 9009542 [details] [diff] [review]
bug1488769-beta.patch

Fix for a new 63 feature, approved for 63 beta 7, thanks.

	Eliza Balazs [:ebalazs_]
	Comment 10 • 6 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/2c477d1661b2

	Timea Zsoldos [:zstimi/tzsoldos], Desktop QA
	Comment 11 • 6 years ago

I have reproduced this issue using Firefox 64.0a1 (2018.09.05) on Ubuntu 16.04 x86.
I can confirm this issue is fixed, I verified using Firefox 63.0b7 and 64.0a1 -latest nightly on Ubuntu 16.04 x86, Windows  7 x64 and Mac OS X 10.13.