Annotate UA Widget Shadow Root in DevTools Inspector

RESOLVED FIXED in Firefox 65

Status

P2
normal
RESOLVED FIXED
7 months ago
4 months ago

People

(Reporter: timdream, Assigned: jdescottes)

Tracking

(Blocks: 3 bugs)

unspecified
Firefox 65
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(4 attachments)

Inspector currently considers UA Widget Shadow Root as normal, closed, Shadow Root. It's probably less confusing to the developers if

1) It's hidden behind a pref like devtools.inspector.showAllAnonymousContent
2) It is marked as "UA Shadow Root" or "UA Widget" or "generated" when shown.

We don't do (2) even for XBL anonymous content and native anonymous content though.
Agreed with (1) - we should hide this for most users.
Blocks: 1478311
Once Bug 1431255 lands, this should be visible with `data:text/html,<video controls />`

Comment 3

7 months ago
I agree with (1) too, possibly calling the pref something like `devtools.inspector.showUserAgentShadowRoots` or something.
Priority: -- → P3
(Assignee)

Comment 4

7 months ago
Chrome has a DevTools preference for this, and when enabled they show them as "#shadow-root (user agent)". I don't think we need to expose this in our Settings panel here.

As additional information, the shadowdom UA widgets are currently disabled even though Bug 1431255 landed. You need to flip dom.ua_widget.enabled to true to enable them.

The pref will be flipped to true for NIGHTLY in https://bugzilla.mozilla.org/show_bug.cgi?id=1484048
As long as it is Nightly-only I don't think we _have_ to fix it for 63, but nevertheless we should address this soon.
(Assignee)

Comment 5

7 months ago
Tim: can you confirm there is no plan to turn on dom.ua_widget.enabled on beta and release in 63?
Flags: needinfo?(timdream)
(In reply to Julian Descottes [:jdescottes][:julian] from comment #5)
> Tim: can you confirm there is no plan to turn on dom.ua_widget.enabled on
> beta and release in 63?

No. I am hoping to enable it on Nightly 64.
Flags: needinfo?(timdream)

Updated

6 months ago
Blocks: 1484272
No longer blocks: 1478311
Priority: P3 → P2
(Assignee)

Comment 7

6 months ago
Is there a way to differentiate a regular closed shadow root from a user-agent shadowroot?
Flags: needinfo?(timdream)
I didn't expose the getter to JS, just the setter. It should be easy to expose a method on ShadowRoot and have it return ShadowRoot#mIsUAWidget.
Flags: needinfo?(timdream)
(Assignee)

Updated

6 months ago
Priority: P2 → P1
(Assignee)

Updated

6 months ago
Priority: P1 → P2
(Assignee)

Comment 11

5 months ago
Tim: we are checking if we should prioritize this bug in Fx64. Is there a chance that UA widgets will be flipped beyond Nightly and will ride the trains in 64? Can it slip to 65?
Flags: needinfo?(timdream)
At the current pace, I guess it would slip to 65. Will comment if otherwise.
Flags: needinfo?(timdream)
(Assignee)

Updated

5 months ago
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
(Assignee)

Comment 13

5 months ago
Tim, I have a basic implementation ready but I have a couple questions:
- is there an added value to use a new pref instead of `devtools.inspector.showAllAnonymousContent`?
- is there any use case where a UA widget might have children that should be visible to a developer? Right now I simply return an empty children list as soon as I detect a UA widget.
Flags: needinfo?(timdream)
(Assignee)

Updated

5 months ago
Blocks: 1503825
(Assignee)

Updated

5 months ago
Blocks: 1503828
(In reply to Julian Descottes [:jdescottes][:julian] from comment #13)
> Tim, I have a basic implementation ready but I have a couple questions:
> - is there an added value to use a new pref instead of
> `devtools.inspector.showAllAnonymousContent`?

I don't think there is other than naming, UA Widgets aren't exactly "anonymous content".

> - is there any use case where a UA widget might have children that should be
> visible to a developer? Right now I simply return an empty children list as
> soon as I detect a UA widget.

UA Widget Shadow DOM should be visible to the developer (when pref'd on) for Firefox development.

Over bug 1425874 I have over implemented <marquee> with UA Widget and <slot> so web developers should definitely be able to see the content of <marquee> slotted in.
Flags: needinfo?(timdream)
Attachment #9021771 - Attachment description: Bug 1483660 - Hide UA Widget children in inspector by default;r=ladybenko → Bug 1483660 - Hide UA Widget shadow root in inspector by default;r=ladybenko
(Assignee)

Comment 19

5 months ago
Thanks Tim! I just changed the implementation in order keep regular children visible (don't know why I didn't think about that earlier but anyway).

This means that for the following authored markup

  <video controls>
    <div>some content</div>
  </video>

The markup view now shows

  devtools.inspector.showUserAgentShadowRoots = false:
  <video controls>
    <div>some content</div>
  </video>
 
  devtools.inspector.showUserAgentShadowRoots = true:
  <video controls>
    #shadow-root (closed)
    <div>some content</div>
  </video>
I can verify that the patch does what desired for video controls. Thank you so much!

I am going to build your patch on top of bug 1425874 to see if it behaves correctly.
... and <marquee> work as expected too.
(Assignee)

Comment 22

5 months ago
thanks for testing Tim!
No worries, thank you for working on this. There is actually an improvement over XBL <marquee> since we would hide the web content child together with XBL anonymous content, while in UA Widget we hide the root but continue to list web content under it.
(Assignee)

Comment 24

5 months ago
Will land the first patch and leave open, so that devtools peers can review the other patches and use artifact builds
Keywords: leave-open
Attachment #9021770 - Attachment description: Bug 1483660 - Expose ShadowRoot::isUAWidget() to chrome code;r=timdream → Bug 1483660 - Expose ShadowRoot::isUAWidget() to chrome code;r=smaug

Comment 25

5 months ago
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/648d56b39b4c
Expose ShadowRoot::isUAWidget() to chrome code;r=smaug
(Assignee)

Updated

4 months ago
Keywords: leave-open

Comment 27

4 months ago
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5c5e2473ba3a
Hide UA Widget shadow root in inspector by default;r=ladybenko
https://hg.mozilla.org/integration/autoland/rev/f6ca26f35f11
Add mochitest for UA widgets in inspector;r=ladybenko
https://hg.mozilla.org/integration/autoland/rev/bd0cbc059da3
Add server test for UA widget support in inspector;r=ladybenko

Comment 28

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5c5e2473ba3a
https://hg.mozilla.org/mozilla-central/rev/f6ca26f35f11
https://hg.mozilla.org/mozilla-central/rev/bd0cbc059da3
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox65: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Depends on: 1505035
You need to log in before you can comment on or make changes to this bug.