Bug 164247 - Web Inspector: Shadow DOM scoped styles are missing
Summary: Web Inspector: Shadow DOM scoped styles are missing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-10-31 15:55 PDT by Joseph Pecoraro
Modified: 2016-10-31 18:19 PDT (History)
9 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (18.29 KB, patch)
2016-10-31 16:05 PDT, Joseph Pecoraro
koivisto: review+
koivisto: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2016-10-31 15:55:38 PDT
Summary:
Shadow DOM scoped styles are missing

Test:
<div><div id="host"></div></div>
<script>
(function() {
    let shadowRoot = document.getElementById("host").attachShadow({mode: "open"});
    shadowRoot.innerHTML = `
        <style>
        :host { padding: 20px; }
        div { color: blue; }
        </style>
        <div id="inner">Shadow Content</div>
    `;
})();
</script>

Steps to Reproduce:
1. Inspect #host on test page
2. Show Styles > Rules sidebar
  => Expected to see :host rule in sidebar
3. Inspect #inner on test page
  => Expected to see scoped "div { color: blue }" rule in sidebar
Comment 1 Joseph Pecoraro 2016-10-31 16:05:08 PDT
Created attachment 293484 [details]
[PATCH] Proposed Fix
Comment 2 Radar WebKit Bug Importer 2016-10-31 16:22:32 PDT
<rdar://problem/29035061>
Comment 3 Devin Rousso 2016-10-31 16:38:12 PDT
Comment on attachment 293484 [details]
[PATCH] Proposed Fix

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

> Source/WebCore/css/InspectorCSSOMWrappers.cpp:101
> +    for (unsigned i = 0; i < sheets.size(); ++i) {

Would it not be better to use a range based for-loop here?
Comment 4 Joseph Pecoraro 2016-10-31 17:03:46 PDT
Comment on attachment 293484 [details]
[PATCH] Proposed Fix

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

>> Source/WebCore/css/InspectorCSSOMWrappers.cpp:101
>> +    for (unsigned i = 0; i < sheets.size(); ++i) {
> 
> Would it not be better to use a range based for-loop here?

I tried and got a compiler error that I didn't want to waste time figuring out. The compiler didn't like:

    for (auto* sheet : sheets)

I welcome suggestions, my C++foo is quite poor =)
Comment 5 Antti Koivisto 2016-10-31 17:11:47 PDT
Comment on attachment 293484 [details]
[PATCH] Proposed Fix

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

>>> Source/WebCore/css/InspectorCSSOMWrappers.cpp:101
>>> +    for (unsigned i = 0; i < sheets.size(); ++i) {
>> 
>> Would it not be better to use a range based for-loop here?
> 
> I tried and got a compiler error that I didn't want to waste time figuring out. The compiler didn't like:
> 
>     for (auto* sheet : sheets)
> 
> I welcome suggestions, my C++foo is quite poor =)

for (auto& sheet : sheets)

should work.

> Source/WebCore/inspector/InspectorCSSAgent.cpp:1018
> +static Style::Scope& containingStyleScopeForElement(Element* element)
> +{
> +    if (auto* shadowRoot = element->containingShadowRoot())
> +        return shadowRoot->styleScope();
> +    return element->document().styleScope();
> +}
> +

You should just use Style::Scope::forNode() instead of this.
Comment 6 Matt Baker 2016-10-31 17:14:03 PDT
Comment on attachment 293484 [details]
[PATCH] Proposed Fix

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

>>>> Source/WebCore/css/InspectorCSSOMWrappers.cpp:101
>>>> +    for (unsigned i = 0; i < sheets.size(); ++i) {
>>> 
>>> Would it not be better to use a range based for-loop here?
>> 
>> I tried and got a compiler error that I didn't want to waste time figuring out. The compiler didn't like:
>> 
>>     for (auto* sheet : sheets)
>> 
>> I welcome suggestions, my C++foo is quite poor =)
> 
> for (auto& sheet : sheets)
> 
> should work.

The only requirement for using range-based for loops with class types is that the class have members begin() and end(). WTF::Vector has these members, you just need to write the loop as Antti suggested, or:

for (const auto& sheet : sheets)

for the const-happy.
Comment 7 Joseph Pecoraro 2016-10-31 18:19:47 PDT
<https://trac.webkit.org/changeset/208199>