Closed Bug 1769771 Opened 2 years ago Closed 2 years ago

Dev Edition is not showing ShadowDOM stylesheet in the devtools anymore

Categories

(DevTools :: Inspector, defect, P2)

Firefox 101
defect

Tracking

(firefox-esr91 unaffected, firefox100 unaffected, firefox101+ verified, firefox102+ verified)

VERIFIED FIXED
102 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox100 --- unaffected
firefox101 + verified
firefox102 + verified

People

(Reporter: gooz, Assigned: emilio)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(5 files)

Attached image dev-edition's bug

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:101.0) Gecko/20100101 Firefox/101.0

Steps to reproduce:

click right > inspect on a element which is inside ShadowDOM.
It opens the devtools on the inspector tab.

Actual results:

The stylesheet area is completely empty.

Expected results:

The stylesheet area should display shadowDOM's CSS applied to the selected element.

After using MozRegression, it narrowed down the issue to this commit:
Bug 1644102 - Turn on constructable stylesheets by default. r=edgar,preferences-reviewers,mstriemer

Also tried on a basic random example of shadowDOM online and it was working fine but it's not on the product I'm working on.
Sadly, it's closed source so I can't provide a testcase. If anything, we're using Stencil to build web-components with shadowDOM.

Has Regression Range: --- → yes
Keywords: regression

Set release status flags based on info from the regressing bug 1644102

:emilio, since you are the author of the regressor, bug 1644102, could you take a look?
For more information, please visit auto_nag documentation.

Flags: needinfo?(emilio)

I can reproduce this when inspecting any element on https://stenciljs.com/ (including <html> and <body>).

So, simple examples with constructable stylesheets seem to work, but the layout inspector and such seem to crash in those pages. I have a couple of trivial fixes to make them usable again.

Blocks: 1602535
Flags: needinfo?(emilio)

These should be trivial fixes for the most part, just fixing assumptions
for constructable stylesheets which have no owner node. Since Gecko
internally also has the concept of "associated document", let's just use
that.

We don't have the authored text around for these. We could keep it
around but I'd kinda prefer we didn't have to just for devtools, since
it seems to me we already need to deal with rules not being there
(imagine e.g. an empty style element with all the rules inserted via
script with insertRule()). So returning the empty string should be
reasonable for now...

Writing some tests now...

Assignee: nobody → emilio
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

I think this is worth it, but needs localization changes so splitting
it out.

Depends on D146682

Just testing that the editor and the new label work as expected since
these are the things that I could find broken in my reduced examples.

Severity: -- → S3
Priority: -- → P2
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6d2ce4736498
Improve DevTools support for constructable stylesheets. r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/fffdcb7e72b5
Differentiate inline from constructed stylesheets in rule view. r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/2627e7b4ccb9
Basic constructable stylesheets + devtools tests. r=nchevobbe

Comment on attachment 9277080 [details]
Bug 1769771 - Improve DevTools support for constructable stylesheets. r=nchevobbe,jdescottes

Beta/Release Uplift Approval Request

  • User impact if declined: DevTools in some pages using constructable stylesheets (feature enabled in 101) will misfunction.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: comment 0 + comment 4
  • List of other uplifts needed: none
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It's very scoped to DevTools, and relatively straight-forward / unlikely to break pre-existing working code.
  • String changes made/needed: One string was added for a DevTools specific view on the rule panel. We could avoid that for beta and just use the pre-existing string but...
  • Is Android affected?: No
Attachment #9277080 - Flags: approval-mozilla-beta?
Attachment #9277081 - Flags: approval-mozilla-beta?
Attachment #9277088 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

I managed to reproduce this issue on DevEdition 101.0b9(20220519220322) on macOS 11. Verified as fixed on Nightly 102.0a1(20220520093126) on macOS 11, Windows 10 64-bits and Ubuntu 20.04.

Given that we're going into RC week next week, I think we should avoid uplifting string changes at this point. How terrible would it be to re-use the existing string on the Beta uplift?

Flags: needinfo?(emilio)
Flags: needinfo?(emilio)
Attachment #9277511 - Flags: approval-mozilla-beta?
Attachment #9277081 - Flags: approval-mozilla-beta?

Comment on attachment 9277080 [details]
Bug 1769771 - Improve DevTools support for constructable stylesheets. r=nchevobbe,jdescottes

Approved for 101.0rc1.

Attachment #9277080 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9277511 - Attachment is patch: true
Attachment #9277511 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9277088 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
See Also: → 1770694

I managed to reproduce this issue on Firefox 101.0b9(20220519220322). Verified as fixed on Firefox 101.0b10(20220520222823) - downloaded from treeherder - on macOS 11, Win10 64-bits and Ubuntu 20.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

I can confirm it works now on Nightly 102.0a1 (2022-05-23). Thanks for the quick response and fix. 👍

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: