Bug 96626 – Web Inspector: Group selectors to highlight matched selector in the Styles pane of Elements Panel

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

[Alexander Pavlov (apavlov)]

It would be really helpful if the Styles pane, in the Elements Panel of the Web Inspector, highlighted or displayed in some way which selector was matched in a group selector. On large projects, selectors can be extended many times, leading to large and confusing selectors. When debugging specificity it would be immensely helpful if we could easily see which selector was matched.

[Alexander Pavlov (apavlov)]

Created attachment 164080 [details]
Patch

[Alexander Pavlov (apavlov)]

Created attachment 164081 [details]
[IMAGE] Screenshot of matching group selectors with the patch applied

[Alexander Pavlov (apavlov)]

Upstreaming http://code.google.com/p/chromium/issues/detail?id=145483

[Pavel Feldman]

I would dim non-matching selectors instead.

[Vsevolod Vlasov]

Comment on attachment 164080 [details]
Patch

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

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1154
> +        var selectors = selectorText.split(",").map(trim);

var selectors = selectorText.split(",").map(String.prototype.trim);

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1177
> +            if (matches && node === paneNode)

s/paneNode/this._parentPane.node/

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1178
> +                selectors[selectorIndex] = "<span class=\"selector-matches\">" + selectors[selectorIndex].escapeHTML() + "</span>";

Let's create DOM element and setContent instead.

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1179
> +            if (isLast) {

selectorIndex === selectors.length - 1

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1180
> +                self._selectorElement.innerHTML = selectors.join(", ");

We should use appendChild

[Pavel Feldman]

Comment on attachment 164080 [details]
Patch

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

>> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1180
>> +                self._selectorElement.innerHTML = selectors.join(", ");
> 
> We should use appendChild

+1. Let's agree that any assignment of innerHTML to non-empy string is an automatic r- in the inspector codebase.

[Vsevolod Vlasov]

(In reply to comment #6)
> (From update of attachment 164080 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=164080&action=review
> 
> >> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1180
> >> +                self._selectorElement.innerHTML = selectors.join(", ");
> > 
> > We should use appendChild
> 
> +1. Let's agree that any assignment of innerHTML to non-empy string is an automatic r- in the inspector codebase.

I don't think there is a valid reason to use innerHTML for empty strings.
element.textContent = "" FTW

[Alexander Pavlov (apavlov)]

Created attachment 164143 [details]
Patch

[Alexander Pavlov (apavlov)]

Created attachment 164156 [details]
[IMAGE] Screenshot - take 2

Non-matching selectors are dimmed. When a section is hovered, all non-matching selectors turn black to improve readability. Inherited element styles' selectors are matched against their respective elements.

[Pavel Feldman]

UI lgtm. Why is html rendered as matching in the inherited from body section?

[Alexander Pavlov (apavlov)]

(In reply to comment #10)
> UI lgtm. Why is html rendered as matching in the inherited from body section?

It's because the mouse pointer is hovering over the section. In that case we un-dim the dimmed selectors to aid users with poor monitors :) and eyesight. Seva said that we should not regress the UI readability in general (that is, it should be possible to view the UI that is not harder to read than it was previously.)

[Pavel Feldman]

I would only make it solid upon selector editing. I think dimmed selector is still very distinguishable.

[Alexander Pavlov (apavlov)]

(In reply to comment #12)
> I would only make it solid upon selector editing. I think dimmed selector is still very distinguishable.

I trust your eyesight! :) Uploading an updated patch shortly.

[Alexander Pavlov (apavlov)]

Created attachment 164359 [details]
Patch

[Alexander Pavlov (apavlov)]

Committed r128746: <http://trac.webkit.org/changeset/128746>

[Pavel Feldman]

I don't see why we need to make 2 additional requests per matched selectors call. We should change the protocol to return ready-to-use data instead.

[Alexander Pavlov (apavlov)]

(In reply to comment #16)
> I don't see why we need to make 2 additional requests per matched selectors call. We should change the protocol to return ready-to-use data instead.

Are you suggesting to send indices of matched selectors in groups up-front?

[Pavel Feldman]

When sending rules, we can send selectors in the model form (array of entities withe the flag responsible for those matching).

[Alexander Pavlov (apavlov)]

*** Bug 31211 has been marked as a duplicate of this bug. ***