Enable STYLESHEET resource type by default
Categories
(DevTools :: Style Editor, task, P3)
Tracking
(Fission Milestone:M7a, firefox89 fixed)
Tracking | Status | |
---|---|---|
firefox89 | --- | fixed |
People
(Reporter: Honza, Assigned: nchevobbe)
References
Details
(Whiteboard: dt-fission-m3-mvp)
Attachments
(5 files)
Thew new STYLESHEET resource and related Fission logic/code path should be enabled by default (we should stop using the pref entirely)
See server Watcher:
Honza
Comment 1•4 years ago
|
||
Tracking dt-fission-m3-mvp bugs for Fission M8 (blocking Release channel experiment, but not Beta experiment).
Comment 2•4 years ago
|
||
Most of the STYLESHEET resource landed in bug 1644193 and last work was on bug 1664941.
Bug 1664941's dependencies already highlights followups and at least one breakage, bug 1673829.
A try run to see if tests are still failing when we turn this on:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ab75317bc633f837164700fe6fe2f8257824b87
Comment 3•4 years ago
|
||
Moving dt-fission-m3-mvp bugs from Fission M8 to M7 (blocking Beta experiment).
Comment 4•4 years ago
|
||
I'm note sure this code works:
https://searchfox.org/mozilla-central/source/devtools/client/fronts/style-rule.js#113-119
As I don't think any front will have resourceWatcher
attribute set.
(This is actually a framework limitation.)
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 5•4 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #4)
I'm note sure this code works:
https://searchfox.org/mozilla-central/source/devtools/client/fronts/style-rule.js#113-119
As I don't think any front will haveresourceWatcher
attribute set.
(This is actually a framework limitation.)
we do set the resourceWatcher
here: https://searchfox.org/mozilla-central/rev/3ff133d19f87da2ba01ade55d6e7fdf0fc28b08c/devtools/client/inspector/inspector.js#277
Comment 6•4 years ago
|
||
Tracking for Fission M8 milestone (Release experiment).
Assignee | ||
Comment 7•4 years ago
|
||
At the moment, when enabling the stylesheet resource watcher on the server there were a few issues.
The main one being a race when doing a target switch, when the client didn't yet
started watching for the needed resource. This caused an issue in the inspector
codepath where we tried to retrieve rules, as the pageStyle and ruleStyle actors
where both using the StylesheetWatcher to do some operation.
In order to not depend on the watcher status, we move most of the logic of the
StylesheetWatcher that wasn't purely watcher related to a StyleSheetsManager, that
listen for new stylesheet being created or modified, and emit event when it's the case,
so the StylesheetWatcher can listen to them.
Some methods are cleaned up, renamed, we add jsdoc where it's missing, and we
also simplify the main map where we store the stylesheet to not have them in
an object (since this was the only property in it).
StyleSheetManagers are kept in a Map keyed by target actor, and a function is exported
so we can clean the Map from the outside.
Assignee | ||
Comment 9•4 years ago
|
||
Enabling the support for stylesheet resources highlighted some issue with the
current situation.
A few tests were failing and had to be adapted to pass.
Depends on D106079
Updated•4 years ago
|
Assignee | ||
Comment 10•4 years ago
|
||
This was previously done in the inspector panel initialization phase, after the
inspector front was created, which could lead to races.
Since the pageStyle actor requires stylesheets to being watched (when watcher
support is enabled), we do this in the inspector front, before creating the
page style actor.
We put the resourceWatcher instance on targetFront from resourceWatcher#_onTargetAvailable
so we can use it from everywhere.
Depends on D106079
Assignee | ||
Comment 11•4 years ago
|
||
With the previous patches in this queue, it looks like the
timing is stlightly modified, and on initial load we can
reveive stylesheet resources before the intial dom-loading
event.
Since we're using dom-loading to clear the UI, we might destroy
stylesheets that we shouldn't.
In order to fix that, we clear the UI in onTargetAvailable, which
we do receive before any resources, and set a flag that we
check in dom-loading to avoid clearing the UI if it was already done.
We take this opportunity to clean a few things in the style editor:
- call watchResources once instead of twice.
- in onResourceAvailable, await for all the promises once, outside
the loop so we don't have delays while looping through the resources.
Depends on D109470
Updated•4 years ago
|
Comment 12•4 years ago
|
||
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7a127a2cb052 [devtools] Move logic outside of StyleSheetWatcher to a new StyleSheetManager class. r=ochameau,jdescottes. https://hg.mozilla.org/integration/autoland/rev/01de742151af [devtools] Use the StyleSheetManager in style-related actors. r=ochameau. https://hg.mozilla.org/integration/autoland/rev/830e139a73a0 [devtools] Watch for stylesheet resources from inspector front instead of inspector panel. r=ochameau. https://hg.mozilla.org/integration/autoland/rev/0068e7f023ce [devtools] Don't clear StyleEditor UI on redundant dom-loading event. r=jdescottes,ochameau. https://hg.mozilla.org/integration/autoland/rev/331d6b2d6f1b [devtools] Enable Stylesheet resource server support for content toolbox. r=ochameau
Comment 13•4 years ago
|
||
Backed out for causing failures on test_styles-applied.html.
Backout link: https://hg.mozilla.org/integration/autoland/rev/84ed889aec1619183b601e0c6138314d9e45f0e6
Failure log: https://treeherder.mozilla.org/logviewer?job_id=334523745&repo=autoland&lineNumber=106673
Assignee | ||
Comment 14•4 years ago
|
||
I'm looking, looks like something landed since last time I pushed to try
Assignee | ||
Comment 15•4 years ago
|
||
new TRY push https://treeherder.mozilla.org/jobs?repo=try&revision=e72e46e13c57a38f97290b2df9d3f14d54733350
Comment 16•4 years ago
|
||
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cb5287e6852c [devtools] Move logic outside of StyleSheetWatcher to a new StyleSheetManager class. r=ochameau,jdescottes. https://hg.mozilla.org/integration/autoland/rev/ff26aac6b8a6 [devtools] Use the StyleSheetManager in style-related actors. r=ochameau. https://hg.mozilla.org/integration/autoland/rev/8c0ee03d5bb6 [devtools] Watch for stylesheet resources from inspector front instead of inspector panel. r=ochameau. https://hg.mozilla.org/integration/autoland/rev/1020d2d180ce [devtools] Don't clear StyleEditor UI on redundant dom-loading event. r=jdescottes,ochameau. https://hg.mozilla.org/integration/autoland/rev/441a2c8df39b [devtools] Enable Stylesheet resource server support for content toolbox. r=ochameau
Comment 17•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cb5287e6852c
https://hg.mozilla.org/mozilla-central/rev/ff26aac6b8a6
https://hg.mozilla.org/mozilla-central/rev/8c0ee03d5bb6
https://hg.mozilla.org/mozilla-central/rev/1020d2d180ce
https://hg.mozilla.org/mozilla-central/rev/441a2c8df39b
Updated•4 years ago
|
Description
•