devtools: Implement Style Editor tab#44517
Conversation
eerii
left a comment
There was a problem hiding this comment.
Nice change! We are going to have to add tests for inline and not inline stylesheets.
I tested this (in https://servo.org) and inline style shows, but the other ones fail with NS_ERROR_FILE_UNRECOGNIZED_PATH: Component returned failure code: 0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH) [nsIFile.initWithPath]. The error shows in the Browser Console and as a banner in DevTools.
| }, | ||
| "getText" => { | ||
| let resource_id = msg.get("resourceId").and_then(|v| v.as_str()).unwrap_or(""); | ||
| // Parse the index from the resourceId ("{browsing_context_id}-{index}") |
There was a problem hiding this comment.
Is this format something that the Firefox client expects or something we are using internally?
There was a problem hiding this comment.
[{"resourceId":"server1.conn0.watcher41.process7//windowGlobalTarget2:stylesheet:1",
"disabled":false,
"constructed":false,
"fileName":null,
"href":null,
"isNew":false,
"atRules":[],
"nodeHref":"http://localhost:8000/p.html",
"ruleCount":2,
"sourceMapBaseURL":"http://localhost:8000/p.html",
"sourceMapURL":"",
"styleSheetIndex":1,
"system":false,
"title":null}]]],\
This is how firefox to firefox stylesheet farwarding happens and each stylesheets resource id helps in getting their CSStext content in the getText message to identify a specific index
and instead of naming the resource like the above i thought of a more convenient one )
There was a problem hiding this comment.
Ok, great, thanks for looking into that! :)
Unrelated to this PR, but @atbrakhi, look at this. They are using the long form of the actor id here, it's more evidence that it's important and maybe that's why the workers are failing
There was a problem hiding this comment.
very certain, we need to have it in order to properly support workers. This is another excellent hint indeed. I have added this in meta issue #44257
Signed-off-by: Rover track <rishan.pgowda@gmail.com>
Now its showing the styles for external ones too! |
eerii
left a comment
There was a problem hiding this comment.
Some more comments but it's looking good! I tried it again, and now external style sheets show, even if they say "Error fetching CSS text" on Servo's main page. But that's already a great improvement! I think we can leave it as is for this PR and add a TODO to tackle it in a followup, otherwise this will get too big.
| }, | ||
| "getText" => { | ||
| let resource_id = msg.get("resourceId").and_then(|v| v.as_str()).unwrap_or(""); | ||
| // Parse the index from the resourceId ("{browsing_context_id}-{index}") |
There was a problem hiding this comment.
Ok, great, thanks for looking into that! :)
Unrelated to this PR, but @atbrakhi, look at this. They are using the long form of the actor id here, it's more evidence that it's important and maybe that's why the workers are failing
it works when you test with your local html !! servo/components/script/dom/css/cssstylesheet.rs Lines 367 to 372 in 142247c |
Signed-off-by: Rover track <rishan.pgowda@gmail.com>
resolved :)
The matching of CSS text formatting as that of raw source can be a later improvement. |
eerii
left a comment
There was a problem hiding this comment.
Great! Just a small lint fix and this is ready for landing.
It works great on https://servo.org and some local tests. I did some testing in larger sites like https://m.huaweimossel.com and it seems like we ran into issues with really big CSS files. But I think it's something we can investigate after this initial implementation is merged.
Followups:
- Show source location for style rules, linking to the file in the Style Editor
- Look into issues displaying big CSS files.
- Proper formatting for external CSS files.
| }; | ||
| request.reply_final(&msg)? | ||
| }, | ||
| /// TODO: Fix CSS text formatting of external sheets, match same as source. |
There was a problem hiding this comment.
This should be //, not ///. The latter is a documentation comment reserved for functions and structs.
Signed-off-by: rovertrack <160643895+rovertrack@users.noreply.github.com>
eerii
left a comment
There was a problem hiding this comment.
Thanks you for tackling this! :)




This PR focuses on forwarding the stylesheets and CSS text to the devtools.
watcher gets request for resource-available it registers in
StyleSheetActorand gets the stylesheets.StyleSheetActorasks thescript_threadfor stylesheets for the current document in the pipeline sends back toStyleSheetActorwhich is sent over for the response.when the request comes for the CSS text which is a
getTextrequest it takes in resource id and the stylesheet index, for each stylesheets it check if it is inline it sends it unchanged, if its not inline it reconstruct it according to theCSSRuleseither of them are sent over the protocol to the devtools client and it shows up in the style editor.Added tests for stylesheets resources and stylesheets CSS text content.
Testing: ./mach test-devtools test_stylesheet
Part of: #44315