Closed Bug 1488769 Opened Last year Closed Last year

Slotted nodes not being displayed in a shadow DOM tree if the node is a text

Categories

(DevTools :: Inspector, defect, P2)

defect

Tracking

(firefox63 verified, firefox64 verified)

VERIFIED FIXED
Firefox 64
Tracking Status
firefox63 --- verified
firefox64 --- verified

People

(Reporter: ladybenko, Assigned: ladybenko)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

Attached image screenshot
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.
Component: General → Inspector
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.
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.
Assignee: nobody → balbeza
Status: NEW → ASSIGNED
Priority: -- → P2
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.
Attachment #9008070 - Flags: review+
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
https://hg.mozilla.org/mozilla-central/rev/a647d0bbf366
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
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>
Flags: qe-verify+
Flags: needinfo?(timea.zsoldos)
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
Attachment #9009542 - Flags: review+
Attachment #9009542 - Flags: approval-mozilla-beta?
Comment on attachment 9009542 [details] [diff] [review]
bug1488769-beta.patch

Fix for a new 63 feature, approved for 63 beta 7, thanks.
Attachment #9009542 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(timea.zsoldos)
You need to log in before you can comment on or make changes to this bug.