Open Bug 1250026 Opened 9 years ago Updated 1 year ago

[Style Editor] Identify Inline style sheets

Categories

(DevTools :: Style Editor, enhancement, P3)

Product:
DevTools
Component:
Style Editor
Version:
44 Branch
Type:
enhancement

Tracking

(Not tracked)

People

(Reporter: karlcow, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

list-of-style-sheets.png
(deleted), image/png
Details
Attached image list-of-style-sheets.png (deleted) — 
1. Go to a site with multiple Inline Style Sheets.
2. See the list of inline style sheet with names like <inline style sheet #nn>

It would be good to have a meaningful identifier such as the Selector path

So instead of 
-------------------------
<inline style sheet #12>
66 rules
-------------------------

You could have 
-------------------------
<inline style sheet #12>
.content foo bar.blah
66 rules
-------------------------

And to have this clickable so you can go back to the inspector and look at the inline style sheet in place.
See also bug 1247083
Seems like a really good idea indeed.
Assigning a priority ( Filter on CLIMBING SHOES ) to fit into the devtools backlog.

I can mentor this bug. Not a really easy one, but doable.
Mentor: pbrosset
Priority: -- → P3
See Also: → https://bugzilla.mozilla.org/show_bug.cgi?id=1247083
Hi Patrick,
I'm interested in working on this issue.
However I am not clear on what the outcome should be.
So here is the screenshot with a particular inline style sheet selected - https://snag.gy/VlnyZ2.jpg
What should this change to?
Flags: needinfo?(pbrosset)
So currently in the screenshot. There is

<inline style sheet #3>
8 rules

In this case, it is 

<style>.gl{white-space:nowrap} …</style> in the source code.

* CSS Selector: head > style:nth-child(9)  

  Though this doesn't work in the Inspector to navigate to the right place.
  BUG? Patrick? Maybe something similar to Bug 1327746 (where not every selector type is working in the search bar of the inspector)
  "head > style+style+style" is working

* CSS Path:     html head style
                
  Too generic


So I would replace this <inline style sheet #3> by

    Inline: "head > style+style+style"
    8 rules 

or anything that would work (the "+" style is not good for brevity)
and make it clickable so that it goes to the right place in the CSS.
Probably something like "head > style:nth-of-type(3)" 
(but not working currently in the inspector)



Note that sometimes CSS have unique id. For example 

<inline style sheet #2> 
53 rules 

is

<style data-jiis="cc" id="gstyle">…</style>

    Inline: #gstyle
    53 rules

would work well.

But I let Patrick reply for the rest.
To clarify the requirement for this feature (because I don't think this has been said explicitly): inline stylesheets shown in the style-editor's sidebar should indicate which <style> DOM element they come from in the DOM tree.

Karl is suggesting the idea of creating a unique CSS selector that would indicate this information.

So, if a <style> node is located in a webpage like so:

<html>
  <head>
    <style>...</style>
  </head>
</html>

The the style-editor would show something like this in the sidebar:

---------------
<inline style sheet #1>
html > head > style
3 rules
---------------

So, this asks the question of which CSS selector to create that will identify the <style> node uniquely.

What is the requirement here exactly?
- is it that this piece of information be clickable so you can jump to the inspector at the right place?
- or is it that the piece of information contains enough data for users? In that case, is a selector useful? If yes, which one? One that uniquely identifies the node on the page (which can potentially be complex if there are no attributes). 

Here is a suggestion: why not use the way we represent nodes in other parts of DevTools here? For instance:
- the breadcrumbs widget at the bottom of the inspector has a way of pretty-printing DOM nodes, and each can be clicked to select the corresponding node in the inspector,
- in the console, if you output a DOM node, same thing there, it's shown like <tag#id.class> and you can click to select the node in the inspector
- same in the animation tool
- same in "inherited" section headers in the CSS rules view
- and probably other places.

This is a common pattern that people expect, so it would be consistent.
However, this does not give a uniquely identifiable representation of a given node.
If a node has an ID, that's great, this will be shown. If the node is only <style>, then that's how it's going to appear.
So potentially you would have a long list of <style>, like:

---------------
<inline style sheet #1>
<style> ● 
3 rules
---------------
<inline style sheet #2>
<style> ● 
13 rules
---------------
<inline style sheet #3>
<style> ● 
1 rule
---------------
<inline style sheet #4>
<style> ● 
33 rules
---------------
<inline style sheet #5>
<style> ● 
5 rules
---------------

But at least, each one would be clickable and that would jump to the corresponding node in the inspector.
(I used ● here to represent the little "inspector" icon we usually place next to these node previews in other places in devtools).

If we go for this, we already have code we can easily use to output these. No work needed there except plumbing things together.

However, we'd still need to get the list of ownerNodes for these inline stylesheets. 
For this, you can use the walker actor's getStyleSheetOwnerNode method.
In fact the style-editor already retrieves it here: http://searchfox.org/mozilla-central/rev/dd47bee6468de7e1221b4d006342ad6b9813d0e5/devtools/client/styleeditor/StyleSheetEditor.jsm#600
but it just doesn't store it anywhere useful. So we would have to call this earlier when a stylesheet is being loaded in the editor, and use it to generate the DOM node preview.

Note: we should not do this right from the start because that will otherwise delay the first rendering of the style-editor. Indeed, we'd need to do one more round-trip to the server for each stylesheet being shown. So I would advise doing this asynchronously, a bit later, once the style-editor has finished loading.

To implement the feature, you'll likely need more technical information, but I believe there's enough here to start looking at the code, and experiment.

And before starting, I'd like Karl's feedback on the requirement.
Flags: needinfo?(pbrosset) → needinfo?(kdubost)
Agreed with everything Patrick said.


1. Be able to identify it when possible 
  (I wonder if having the feature will encourage people 
   to label their style with a class or an id, 
   because they know they would see it in the devtools.)
2. Be able to jump to the CSS in place in the inspector.

Basically, given the possibility for the users to understand a bit more of the CSS context. 

Thanks Patrick for eloquently explaining the feature.
Flags: needinfo?(kdubost)
Product: Firefox → DevTools
Severity: normal → enhancement
Priority: P3 → P1
Mentor: patrickbrosset+bugzilla
Severity: normal → S3
Priority: P1 → P3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: