Bug 164247 – Web Inspector: Shadow DOM scoped styles are missing

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

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

[Joseph Pecoraro]

Created attachment 293484 [details]
[PATCH] Proposed Fix

[Radar WebKit Bug Importer]

<rdar://problem/29035061>

[Devin Rousso]

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?

[Joseph Pecoraro]

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 =)

[Antti Koivisto]

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.

[Matt Baker]

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.

[Joseph Pecoraro]

<https://trac.webkit.org/changeset/208199>