Bug 65626 – Web Inspector: Show media queries associated with specific CSS rules

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Zach Leatherman]

This is probably best explained with an example.

Go to http://jquerymobile.com/test/
Inspect div.content-secondary
When the window >= 650px, the width of this element is set to 45%.
When I resize the window < 650px, there is no width set.

There currently is no visual indicator that exists when the width is set to 45% that it is conditional on a media query.  I would like this to be explicit in the web inspector styles.

Maybe something like this in the Styles pane:

@media all and (min-width: 650px){              jqm-docs.css:150
  .content-secondary {                          jqm-docs.css:178
    text-align: left;
    float: left;
    width: 45%;
    background: none;
    border-top: 0;
  }
}

[Alexander Pavlov (apavlov)]

Created attachment 114082 [details]
Patch

[Pavel Feldman]

Comment on attachment 114082 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114082&action=review

Could we get a screenshot for this? r- for a number of small problems.

> Source/WebCore/inspector/Inspector.json:-446
> -                "description": "Enables console domain, sends the messages collected so far to the client by means of the <code>messageAdded</code> notification." 

This change will confuse the version control annotation. Could you not do it please?

> Source/WebCore/inspector/InspectorStyleSheet.cpp:750
> +        for (size_t i = mediaVector.size(); i > 0; --i)

We usually do = size - 1; i >=0; --i

So the order is self media followed by a reversed list (i.e. starting from root?) This is confusing, the list should be either from root to current or from current to root.

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:843
> +            this._mediaTextElement = this.titleElement.createChild("div", "media");

So this section property will iterate over all of the elements and will not be accessed from anywhere else? Can we use local variable instead?

> LayoutTests/inspector/styles/styles-iframe-expected.txt:9
> +[expanded] media="screen"@media screen#main { (styles-iframe.html:6)

A test involving nested medias would be great.

[Alexander Pavlov (apavlov)]

Created attachment 114289 [details]
Patch

[Alexander Pavlov (apavlov)]

A more complete solution has been attached, which enables navigation to media lists in the chain, when possible.

(In reply to comment #2)
> (From update of attachment 114082 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=114082&action=review
> 
> Could we get a screenshot for this? r- for a number of small problems.
> 
> > Source/WebCore/inspector/Inspector.json:-446
> > -                "description": "Enables console domain, sends the messages collected so far to the client by means of the <code>messageAdded</code> notification." 
> 
> This change will confuse the version control annotation. Could you not do it please?

Fixed

> > Source/WebCore/inspector/InspectorStyleSheet.cpp:750
> > +        for (size_t i = mediaVector.size(); i > 0; --i)
> 
> We usually do = size - 1; i >=0; --i

Heh. It's a size_t which is unsigned. Resorted to building an outward-traversal list and reversing it while rendering.

> So the order is self media followed by a reversed list (i.e. starting from root?) This is confusing, the list should be either from root to current or from current to root.

It was not SELF MEDIA but media query of the parent stylesheet, the root one. But I've found out that @import rules can also have media queries, so the traversal is slightly more involved (see the extracted method).

> > Source/WebCore/inspector/front-end/StylesSidebarPane.js:843
> > +            this._mediaTextElement = this.titleElement.createChild("div", "media");
> 
> So this section property will iterate over all of the elements and will not be accessed from anywhere else? Can we use local variable instead?

Fixed.

> > LayoutTests/inspector/styles/styles-iframe-expected.txt:9
> > +[expanded] media="screen"@media screen#main { (styles-iframe.html:6)
> 
> A test involving nested medias would be great.

Done.

[Pavel Feldman]

Comment on attachment 114289 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114289&action=review

Inspector looks good. Please make sure CSS folks are file with the m_lastLine. It looks sane to me.

> Source/WebCore/inspector/InspectorStyleSheet.cpp:136
> +    default:

Removing default will convert it from runtime to compile time error.

[Alexander Pavlov (apavlov)]

Created attachment 114292 [details]
[IMAGE] Screenshot of the Styles sidebar pane with media queries displayed

[Alexander Pavlov (apavlov)]

Created attachment 114455 [details]
Patch

[PATCH] Fix winbot compilability, adjust linked URLs display

[Alexander Pavlov (apavlov)]

Created attachment 114458 [details]
Patch

[PATCH] Re-submit for Windows EWS bot (some weird bot issue occurred)

[Antti Koivisto]

Comment on attachment 114458 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114458&action=review

> Source/WebCore/css/MediaList.h:89
> +    int lastLine() const { return m_lastLine; }
> +    void updateLastLine(int lastLine) { m_lastLine = lastLine; }

Can lastLine be negative? If not, why signed type?

WebKit coding style is to usually name setters as "setFoo".

[Antti Koivisto]

Looks ok otherwise.

[Alexander Pavlov (apavlov)]

(In reply to comment #9)
> (From update of attachment 114458 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=114458&action=review
> 
> > Source/WebCore/css/MediaList.h:89
> > +    int lastLine() const { return m_lastLine; }
> > +    void updateLastLine(int lastLine) { m_lastLine = lastLine; }
> 
> Can lastLine be negative? If not, why signed type?

CSS classes (CSSParser, CSSStyleRule) use "int" to that end, so I decided to stick to the same type.

> WebKit coding style is to usually name setters as "setFoo".

Will fix.

[Alexander Pavlov (apavlov)]

Created attachment 114480 [details]
Patch

Comment from anttik fixed

[Alexander Pavlov (apavlov)]

Committed r99849: <http://trac.webkit.org/changeset/99849>