1776831 - Rule view doesn't refresh when stylesheet is added or modified

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, defect, P2)

Description

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced test-case.

Thank you for helping make Firefox better. If you are reporting a defect, please complete the following:

What were you doing?

See the reduced test-case. The rule view doesn't update properly unless you re-inspect the element.

This was originally reported here, and it breaks some hot-reloading use-cases: https://www.reddit.com/r/firefox/comments/vj1k6n/firefox_devtools_css_state_not_live_updating/

What happened?

Background changes, but the rule view doesn't get updated.

What should have happened?

Rule view gets updated to reflect the new stylesheet.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Screencast of the bug.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I think DevTools should have enough information to refresh the rule view in this case (the applicable state change event is probably firing), but if it doesn't we should figure out a way to inform devtools of stylesheets changing perhaps.

Comment 3

[Simon Pieters [:zcorpan]]

This bug might be the root cause for broken hot-reload with Next13 + Hotwind CSS (and possibly also other frameworks):

Hot reloading of Tailwind CSS in Firefox when you change classes in your code doesn't work.

https://github.com/vercel/next.js/issues/44782 (edit: this was closed as duplicate of https://github.com/vercel/next.js/issues/43396 )

Comment 4

[ddavenport161]

Having the exact same issue. Can we please get some movement on this? I would really like to use Firefox and not a Chromium based browser.

Comment 5

[Nicolas Chevobbe [:nchevobbe][PTO - back on Monday May 13th]]

Looked it up a bit.
So we're indeed receiving the applicable state change, but somehow the stylesheet manager sees it as a new stylesheet, and so we do emit a new STYLESHEET resource.
We should probably fix that, because in the style editor this ends up adding new stylesheets.

Secondly, the rule view doesn't refresh when a new stylesheet is added, hence why we're showing out-of-date data here.
I'll spawn another bug for that, with a "proper" test case (actually adding a new stylesheet)
This can be seen on this page, selecting the body and clicking the button in the page: data:text/html,<meta charset=utf8><button>Add stylesheet</button><script>document.addEventListener("click", () => { s = document.createElement("style"); s.textContent = 'body{ background: tomato }'; document.head.append(s); })</script>

Comment 6

[Nicolas Chevobbe [:nchevobbe][PTO - back on Monday May 13th]]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #5)

Looked it up a bit.
So we're indeed receiving the applicable state change, but somehow the stylesheet manager sees it as a new stylesheet, and so we do emit a new STYLESHEET resource.
We should probably fix that, because in the style editor this ends up adding new stylesheets.

Filed Bug 1847165 for this

Comment 7

[Nicolas Chevobbe [:nchevobbe][PTO - back on Monday May 13th]]

Attachment: Bug 1776831 - [devtools] Refresh rules view on STYLESHEET resource available. r=jdescottes.

Comment 8

[Nicolas Chevobbe [:nchevobbe][PTO - back on Monday May 13th]]

Attachment: Bug 1776831 - [devtools] Turn RuleViewTool into an ES6 class. r=#devtools-reviewers.

Depends on D185393

Comment 9

[Nicolas Chevobbe [:nchevobbe][PTO - back on Monday May 13th]]

Attachment: Bug 1776831 - [devtools] Control RuleViewTool with an AbortController. r=#devtools-reviewers.

Depends on D185557

Comment 10

[Nicolas Chevobbe [:nchevobbe][PTO - back on Monday May 13th]]

Attachment: Bug 1776831 - [devtools] Refresh ruleview when inspector is selected. r=#devtools-reviewers.

We are already refreshing the view when the sidebar is selected (so for example,
going from the layout to the rules view), but we were not refreshing it when
selecting the panel (so for example, going from the styleeditor to the inspector,
while the rule view is the active sidebar panel).

While writing a test, I realized CssRuleView#isPanelVisible was always returning
true when the 3 pane mode is enabled, even if the inspector isn't the active tool,
which is incorrect, so I fixed it.

A test is added to ensure we covers this, by adding a stylesheet from the style editor.

Depends on D185558

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c306b234b8ca
[devtools] Refresh rules view on STYLESHEET resource available. r=jdescottes,devtools-reviewers.
https://hg.mozilla.org/integration/autoland/rev/f77f0a2577a1
[devtools] Turn RuleViewTool into an ES6 class. r=devtools-reviewers,jdescottes.
https://hg.mozilla.org/integration/autoland/rev/bba9f83f2d27
[devtools] Control RuleViewTool with an AbortController. r=devtools-reviewers,jdescottes.
https://hg.mozilla.org/integration/autoland/rev/e4dbab6b2e3f
[devtools] Refresh ruleview when inspector is selected. r=devtools-reviewers,jdescottes.

Comment 12

[Noemi Erli[:noemi_erli]]

Backed out 4 changesets (Bug 1776831) for causing failures in browser_rules_add-rule-and-remove-style-node.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=DfSGBvbDSyeeOmdYUA1RSw.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=e4dbab6b2e3f1c5ec3d4d73dd7c6eb0d51dd05f7

Log: https://treeherder.mozilla.org/logviewer?job_id=425651932&repo=autoland&lineNumber=7888

Backout: https://hg.mozilla.org/integration/autoland/rev/efb44db563b1abcb74959e12c6ce8e577cd213f2

Comment 13

[Nicolas Chevobbe [:nchevobbe][PTO - back on Monday May 13th]]

I'm investigating

Comment 14

[Nicolas Chevobbe [:nchevobbe][PTO - back on Monday May 13th]]

Attachment: Bug 1776831 - [devtools] Fix race condition when adding new rule from the UI. r=#devtools-reviewers.

As ruleview-refreshed event can be triggered from different
actions, we might get an event before the rule was actually
added.
To fix this, wait for ruleview-refreshed until the added rule
can be find in this._elementStyle.rules.

Depends on D185560

Comment 15

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1776831 - add userAdded down the line

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e32c7f26ae23
[devtools] Refresh rules view on STYLESHEET resource available. r=jdescottes,devtools-reviewers.
https://hg.mozilla.org/integration/autoland/rev/6c3e8559f1a4
[devtools] Turn RuleViewTool into an ES6 class. r=devtools-reviewers,jdescottes.
https://hg.mozilla.org/integration/autoland/rev/9381132250d9
[devtools] Control RuleViewTool with an AbortController. r=devtools-reviewers,jdescottes.
https://hg.mozilla.org/integration/autoland/rev/2dace6b56663
[devtools] Refresh ruleview when inspector is selected. r=devtools-reviewers,jdescottes.
https://hg.mozilla.org/integration/autoland/rev/2750553de9a1
[devtools] Fix race condition when adding new rule from the UI. r=devtools-reviewers,ochameau.

Comment 17

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/e32c7f26ae23
https://hg.mozilla.org/mozilla-central/rev/6c3e8559f1a4
https://hg.mozilla.org/mozilla-central/rev/9381132250d9
https://hg.mozilla.org/mozilla-central/rev/2dace6b56663
https://hg.mozilla.org/mozilla-central/rev/2750553de9a1