Bug 138185 - Web Inspector: I do not expect to see the same rule multiple times in styles sidebar
Summary: Web Inspector: I do not expect to see the same rule multiple times in styles ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-10-29 14:23 PDT by Joseph Pecoraro
Modified: 2015-01-13 13:44 PST (History)
5 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (3.97 KB, patch)
2014-12-01 17:12 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (3.98 KB, patch)
2014-12-01 17:14 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2014-10-29 14:23:52 PDT
* SUMMARY
When a rule with multiple selectors matching the current selected element, the rule shows up multiple times in the sidebar.

* TEST

    <style>*, h1, .a, h1.a, body > #foo.a.b { color: green; }</style>
    <h1 id="foo" class="a b c">Hello World</h1>

* STEPS TO REPRODUCE
1. Inspect h1#foo on test page
2. Show styles sidebar
  => See the one rule 5 times

* NOTES
It is not too unreasonable to show this rule multiple times. For instance in this test:

    <style>
    h1, h1.a { color: blue }
    .a { color: red; }
    </style>
    <h1 class="a">Test</h1>
  
Which will show the fallback list as:

    Rule 1 (h1.a) -> Rule 2 (.a) -> Rule 1 (a)

However, in the case where the same rule matches multiple times, it seems the case of higher specificity should show, and the others do not need to.

* OTHER BROWSERS
- Chrome matches Safari now and shows the rule multiple times
- Firefox only shows the rule once

Thoughts?
Comment 1 Radar WebKit Bug Importer 2014-10-29 14:24:24 PDT
<rdar://problem/18816349>
Comment 2 Joseph Pecoraro 2014-10-29 14:24:46 PDT
Probably a dup of:
https://bugs.webkit.org/show_bug.cgi?id=113476

I rather like the idea of reducing clutter.
Comment 3 Timothy Hatcher 2014-10-29 14:27:03 PDT
I debated this when I redid the sidebar. It can be important when each selector has different specificity and other rules interleave with it. Different CSS properties could/will be overridden in each instance of the rule with multiple selectors.
Comment 4 Timothy Hatcher 2014-10-29 14:27:39 PDT
Maybe we can show them but in some new collapsed state?
Comment 5 Timothy Hatcher 2014-10-29 14:29:14 PDT
Or only show them if interleaving happens. Otherwise it collapses into one instance.
Comment 6 Joseph Pecoraro 2014-12-01 17:09:39 PST
I think the deduplication really helps simplify the sidebar.

One case I hadn't thought of that this helps is inherited styles. Take this test:

    <html>
    <head>
    <style>
    /* simplified reset.css */
    html, body, div, p { margin:0; padding:0; border:0; font-style: inherit; }
    body { line-height:1.5; }
    /* page styles */
    p { margin: 0 0 1.5em; }
    </style>
    </head>
    <body><div><p>Test</p></div></body>
    </html>

Inspecting the <p> you will see the first reset rule 4 times total because of the "font-style: inherit" for the p, div, body, html. With de-duplication you only see it once, up above with high specificity and never in the inherited section. If there was in fact a font style it inherits, that would be listed in the inherited section, seriously reducing clutter.
Comment 7 Joseph Pecoraro 2014-12-01 17:12:36 PST
Created attachment 242365 [details]
[PATCH] Proposed Fix

We should probably also include the complete fallback list in the Computed Rules section with the complete cascade per-property like Firefox/Chrome if there is ever a case where interleaving matters.
Comment 8 Joseph Pecoraro 2014-12-01 17:13:51 PST
Comment on attachment 242365 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/ChangeLog:18
> +        Remove duplicates from the ordered list of selectors.

s/selectors/CSSStyleDeclarations/
Comment 9 Joseph Pecoraro 2014-12-01 17:14:26 PST
Created attachment 242367 [details]
[PATCH] Proposed Fix
Comment 10 WebKit Commit Bot 2015-01-13 13:44:20 PST
Comment on attachment 242367 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 242367

Committed r178373: <http://trac.webkit.org/changeset/178373>
Comment 11 WebKit Commit Bot 2015-01-13 13:44:24 PST
All reviewed patches have been landed.  Closing bug.