Skip to content

Commit

Permalink
Web Inspector: Show parent style rules for nested style rules in Styl…
Browse files Browse the repository at this point in the history
…es sidebar

https://bugs.webkit.org/show_bug.cgi?id=250088
rdar://100522930

Reviewed by Antti Koivisto.

Add basic support for viewing and editing parent style rule selectors for nested style rules. This uses the same
mechanism we already use for showing parent `@rule`s, including support for editing.

* LayoutTests/inspector/css/getMatchesStylesForNodeNestingStyleGrouping-expected.txt: Added.
* LayoutTests/inspector/css/getMatchesStylesForNodeNestingStyleGrouping.html: Added.

* LayoutTests/inspector/css/setGroupingHeaderText-expected.txt:
* LayoutTests/inspector/css/setGroupingHeaderText.html:
- Add test cases for modifying a parent Style rule's selector via a Grouping on a child nested rule.

* Source/JavaScriptCore/inspector/protocol/CSS.json:
- Add new "StyleRule" grouping type to represent parent style rules.

* Source/WebCore/WebCore.xcodeproj/project.pbxproj:

* Source/WebCore/css/CSSStyleRule.cpp:
(WebCore::CSSStyleRule::CSSStyleRule):
(WebCore::CSSStyleRule::length const):
(WebCore::CSSStyleRule::item const):
(WebCore::CSSStyleRule::cssRules const):
* Source/WebCore/css/CSSStyleRule.h:
- Add support for getting the nested rules inside a CSSStyleRule.

* Source/WebCore/css/StyleRule.cpp:
(WebCore::StyleRuleBase::createCSSOMWrapper const):
* Source/WebCore/css/StyleRule.h:
- Support creating a wrapper with a CSSStyleRule as a parent.

* Source/WebCore/css/parser/CSSParser.cpp:
(WebCore::CSSParser::parseSelector):
* Source/WebCore/css/parser/CSSParser.h:
- Allow callers to enable nesting syntax for parsing a selector, which is done to verify a selector is valid by Web
Inspector when editing a nested rule's selector.

* Source/WebCore/inspector/InspectorStyleSheet.cpp:
(WebCore::flatteningStrategyForStyleRuleType):
- Style rules can now contain rules, so get rid of the `Commit` strategy and use the existing `CommitSelfThenChildren`
for style rules instead.

(WebCore::isValidRuleHeaderText):
- Pass through the nesting mode for parsing selectors.

(WebCore::protocolGroupingTypeForStyleRuleType):
(WebCore::flattenSourceData):
- Get rid of `Commit` strategy.

(WebCore::StyleSheetHandler::startRuleHeader):
- It is no longer invalid to encounter the start of a style rule before encountering the end of the previous style rule.

(WebCore::asCSSRuleList):

(WebCore::InspectorStyleSheet::buildArrayForGroupings):
- Start with the parent of the passed CSSRule, otherwise every style rule will include itself as a grouping.

(WebCore::isNestedContext):
(WebCore::InspectorStyleSheet::setRuleHeaderText):
- Nested rules should allow relevant syntax for selectors.

(WebCore::InspectorStyleSheet::collectFlatRules):

* Source/WebCore/style/InspectorCSSOMWrappers.cpp:
(WebCore::Style::InspectorCSSOMWrappers::collect):
- Eagerly create CSSOM wrappers for nested rules.

* Source/WebInspectorUI/UserInterface/Models/CSSGrouping.js:
(WI.CSSGrouping.prototype.get isStyle):
(WI.CSSGrouping.prototype.get prefix):
(WI.CSSGrouping):
- Nested rules don't have a prefix like `@rule`s do, so provide a null prefix.

* Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._renderGroupings):
- Support groupings without a prefix by not prepending the prefix.

Canonical link: https://commits.webkit.org/258555@main
  • Loading branch information
patrickangle committed Jan 6, 2023
1 parent 0a3c1ae commit b5cf192
Show file tree
Hide file tree
Showing 19 changed files with 342 additions and 51 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
Tests for the CSS.getMatchedStyleForNode command and container rule groups.


== Running test suite: CSS.getMatchedStyleForNode.NestingStyleGrouping
-- Running test case: CSS.getMatchedStyleForNode.NestingStyleGrouping.NormalProperty
PASS: Should have 1 authored rules.
- Testing rule #0
PASS: Selector text should be "#outerA".
PASS: "color" property value should be "red".
PASS: Rule should have no groupings.

-- Running test case: CSS.getMatchedStyleForNode.NestingStyleGrouping.NestedRulePropertyWithImplicitAmpersand
PASS: Should have 2 authored rules.
- Testing rule #0
PASS: Selector text should be ".innerA".
PASS: "color" property value should be "green".
PASS: Rule should have 1 grouping(s).
PASS: Grouping 0 should have a type of "style-rule".
PASS: Grouping 0 should have a text of "#outerA".
- Testing rule #1
PASS: Selector text should be "#outerA".
PASS: "color" property value should be "red".
PASS: Rule should have no groupings.

-- Running test case: CSS.getMatchedStyleForNode.NestingStyleGrouping.NestedRulePropertyWithImplicitAmpersand
PASS: Should have 1 authored rules.
- Testing rule #0
PASS: Selector text should be ".outer:not(&)".
PASS: "color" property value should be "blue".
PASS: Rule should have 1 grouping(s).
PASS: Grouping 0 should have a type of "style-rule".
PASS: Grouping 0 should have a text of "#outerA".

Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
<!DOCTYPE html>
<html>
<head>
<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
<script>
function test()
{
let suite = InspectorTest.createAsyncSuite("CSS.getMatchedStyleForNode.NestingStyleGrouping");

function expectRuleAtIndex(rules, index, {selectorText, colorPropertyValue, groupings})
{
InspectorTest.log(`- Testing rule #${index}`);

let rule = rules[index];
InspectorTest.expectEqual(rule.selectorText, selectorText, `Selector text should be "${selectorText}".`);
InspectorTest.expectEqual(rule.style.propertyForName("color").value, colorPropertyValue, `"color" property value should be "${colorPropertyValue}".`);

if (!groupings) {
InspectorTest.expectEmpty(rule.groupings, "Rule should have no groupings.");
return;
}

InspectorTest.expectEqual(rule.groupings.length, groupings.length, `Rule should have ${groupings.length} grouping(s).`);

for (let i = 0; i < groupings.length; ++i) {
InspectorTest.expectEqual(rule.groupings[i].type, groupings[i].type, `Grouping ${i} should have a type of "${groupings[i].type}".`);

if (groupings[i].text)
InspectorTest.expectEqual(rule.groupings[i].text, groupings[i].text, `Grouping ${i} should have a text of "${groupings[i].text}".`);
else
InspectorTest.expectNull(rule.groupings[i].text, `Grouping ${i} should not have any text.`);
}
}

function addTestCase({name, description, selector, expectedAuthoredRuleCount, authoredRulesHandler})
{
suite.addTestCase({
name,
description,
async test() {
let documentNode = await WI.domManager.requestDocument();
let nodeId = await documentNode.querySelector(selector);
let domNode = WI.domManager.nodeForId(nodeId);
InspectorTest.assert(domNode, `Should find DOM Node for selector '#item'.`);

let domNodeStyles = WI.cssManager.stylesForNode(domNode);
InspectorTest.assert(domNodeStyles, `Should find CSS Styles for DOM Node.`);
await domNodeStyles.refresh();

let flatInheritedRules = domNodeStyles.inheritedRules.flatMap((inheritedRuleInfo) => inheritedRuleInfo.matchedRules);

let authoredRules = [...domNodeStyles.matchedRules, ...flatInheritedRules].filter((rule) => rule.type === WI.CSSStyleSheet.Type.Author);
InspectorTest.expectEqual(authoredRules.length, expectedAuthoredRuleCount, `Should have ${expectedAuthoredRuleCount} authored rules.`);
authoredRulesHandler(authoredRules);
},
});
}

addTestCase({
name: "CSS.getMatchedStyleForNode.NestingStyleGrouping.NormalProperty",
description: "Non-nested properties should be visible even when nested style rules are present.",
selector: "#outerA",
expectedAuthoredRuleCount: 1,
authoredRulesHandler(rules) {
expectRuleAtIndex(rules, 0, {
selectorText: "#outerA",
colorPropertyValue: "red",
});
}
});

addTestCase({
name: "CSS.getMatchedStyleForNode.NestingStyleGrouping.NestedRulePropertyWithImplicitAmpersand",
description: "Nested rules with an implicit ampersand should have a grouping representing the outer style rule.",
selector: "#outerA .innerA",
expectedAuthoredRuleCount: 2,
authoredRulesHandler(rules) {
expectRuleAtIndex(rules, 0, {
selectorText: ".innerA",
colorPropertyValue: "green",
groupings: [
{type: WI.CSSGrouping.Type.StyleRule, text: "#outerA"},
],
});
expectRuleAtIndex(rules, 1, {
selectorText: "#outerA",
colorPropertyValue: "red",
});
}
});

addTestCase({
name: "CSS.getMatchedStyleForNode.NestingStyleGrouping.NestedRulePropertyWithImplicitAmpersand",
description: "Nested rules with an explicit ampersand should have a grouping representing the outer style rule, even when the outer rule's style does not apply to the element.",
selector: "#outerB",
expectedAuthoredRuleCount: 1,
authoredRulesHandler(rules) {
expectRuleAtIndex(rules, 0, {
selectorText: ".outer:not(&)",
colorPropertyValue: "blue",
groupings: [
{type: WI.CSSGrouping.Type.StyleRule, text: "#outerA"},
],
});
}
});

suite.runTestCasesAndFinish();
}
</script>
<style>
#outerA {
color: red;

.innerA {
color: green;
}

.outer:not(&) {
color: blue;
}
}
</style>
</head>
<body onload="runTest()">
<p>Tests for the CSS.getMatchedStyleForNode command and container rule groups.</p>
<div id="outerA" class="outer">
<div class="innerA"></div>
</div>
<div id="outerB" class="outer">
</div>
</body>
</html>
20 changes: 20 additions & 0 deletions LayoutTests/inspector/css/setGroupingHeaderText-expected.txt
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,23 @@ PASS: Setting text should succeed.
PASS: Text should change.
PASS: Text should change on the same grouping attached to a different rule.

-- Running test case: CSS.setGroupingHeaderText.StyleRule
PASS: Should have 2 authored rule.
PASS: Should find grouping of type 'style-rule'.
Setting text to '#test, #test2'.
PASS: Setting text should succeed.
PASS: Text should change.
PASS: Text should change on the same grouping attached to a different rule.
Setting text to '#test {} #test'.
PASS: Setting text should fail.
PASS: Text should not change.
PASS: Text should not change on the same grouping attached to a different rule.
Setting text to ''.
PASS: Setting text should fail.
PASS: Text should not change.
PASS: Text should not change on the same grouping attached to a different rule.
Setting text to '#test'.
PASS: Setting text should succeed.
PASS: Text should change.
PASS: Text should change on the same grouping attached to a different rule.

32 changes: 29 additions & 3 deletions LayoutTests/inspector/css/setGroupingHeaderText.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
{
let suite = InspectorTest.createAsyncSuite("CSS.setGroupingHeaderText");

function addTestCase({name, description, groupingType, subtestCases})
function addTestCase({name, description, selector, groupingType, subtestCases})
{
suite.addTestCase({
name,
description,
async test() {
let documentNode = await WI.domManager.requestDocument();
let nodeId = await documentNode.querySelector("#test");
let nodeId = await documentNode.querySelector(selector);
let domNode = WI.domManager.nodeForId(nodeId);
InspectorTest.assert(domNode, `Should find DOM Node for selector '#test'.`);

Expand Down Expand Up @@ -60,6 +60,7 @@
addTestCase({
name: "CSS.setGroupingHeaderText.MediaRule",
description: "@media rules should be mutable and should only accept syntactically correct text.",
selector: "#test",
groupingType: WI.CSSGrouping.Type.MediaRule,
subtestCases: [
{ text: "not print" },
Expand All @@ -78,6 +79,7 @@
addTestCase({
name: "CSS.setGroupingHeaderText.SupportsRule",
description: "@supports rules should be mutable and should only accept syntactically correct text.",
selector: "#test",
groupingType: WI.CSSGrouping.Type.SupportsRule,
subtestCases: [
{ text: "(color: purple)" },
Expand All @@ -98,6 +100,7 @@
addTestCase({
name: "CSS.setGroupingHeaderText.LayerRule",
description: "@layer rules should be mutable and should only accept syntactically correct text.",
selector: "#test",
groupingType: WI.CSSGrouping.Type.LayerRule,
subtestCases: [
{ text: "howdy" },
Expand All @@ -112,6 +115,7 @@
addTestCase({
name: "CSS.setGroupingHeaderText.ContainerRule",
description: "@container rules should be mutable and should only accept syntactically correct text.",
selector: "#test",
groupingType: WI.CSSGrouping.Type.ContainerRule,
subtestCases: [
{ text: "name (min-width: 42px)" },
Expand All @@ -127,6 +131,18 @@
],
});

addTestCase({
name: "CSS.setGroupingHeaderText.StyleRule",
description: "Parent style rules for a nested style rule should be mutable and should only accept syntactically correct text.",
selector: "#nested",
groupingType: WI.CSSGrouping.Type.StyleRule,
subtestCases: [
{ text: "#test, #test2" },
{ text: "#test {} #test", expectsFailure: true },
{ text: "", expectsFailure: true },
],
});

suite.runTestCasesAndFinish();
}
</script>
Expand All @@ -141,6 +157,14 @@
@container (min-width: 1px) {
#test {
color: lawngreen;

#nested {
color: rebeccapurple;
}

.nested {
color: seagreen;
}
}

.test {
Expand All @@ -154,6 +178,8 @@
</head>
<body onload="runTest()">
<p>Tests for the CSS.setGroupingHeaderText command.</p>
<div id="test" class="test"></div>
<div id="test" class="test">
<div id="nested" class="nested"></div>
</div>
</body>
</html>
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/inspector/protocol/CSS.json
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,8 @@
"type": "object",
"description": "CSS @media (as well as other users of media queries, like @import, <style>, <link>, etc.), @supports, and @layer descriptor.",
"properties": [
{ "name": "type", "type": "string", "enum": ["media-rule", "media-import-rule", "media-link-node", "media-style-node", "supports-rule", "layer-rule", "layer-import-rule", "container-rule"], "description": "Source of the media query: \"media-rule\" if specified by a @media rule, \"media-import-rule\" if specified by an @import rule, \"media-link-node\" if specified by a \"media\" attribute in a linked style sheet's LINK tag, \"media-style-node\" if specified by a \"media\" attribute in an inline style sheet's STYLE tag, \"supports-rule\" if specified by an @supports rule, \"layer-rule\" if specified by an @layer rule, \"container-rule\" if specified by an @container rule." },
{ "name": "ruleId", "$ref": "CSSRuleId", "optional": true, "description": "The CSS rule identifier for the `@rule` (absent for non-editable grouping rules). In CSSOM terms, this is the parent rule of either the previous Grouping for a CSSRule, or of a CSSRule itself."},
{ "name": "type", "type": "string", "enum": ["media-rule", "media-import-rule", "media-link-node", "media-style-node", "supports-rule", "layer-rule", "layer-import-rule", "container-rule", "style-rule"], "description": "Source of the media query: \"media-rule\" if specified by a @media rule, \"media-import-rule\" if specified by an @import rule, \"media-link-node\" if specified by a \"media\" attribute in a linked style sheet's LINK tag, \"media-style-node\" if specified by a \"media\" attribute in an inline style sheet's STYLE tag, \"supports-rule\" if specified by an @supports rule, \"layer-rule\" if specified by an @layer rule, \"container-rule\" if specified by an @container rule, \"style-rule\" if specified by a CSSStyleRule containing the rule inside this grouping." },
{ "name": "ruleId", "$ref": "CSSRuleId", "optional": true, "description": "The CSS rule identifier for the `@rule` (absent for non-editable grouping rules) or the nesting parent style rule's selector. In CSSOM terms, this is the parent rule of either the previous Grouping for a CSSRule, or of a CSSRule itself."},
{ "name": "text", "type": "string", "optional": true, "description": "Query text if specified by a @media, @supports, or @container rule. Layer name (or not present for anonymous layers) for @layer rules." },
{ "name": "sourceURL", "type": "string", "optional": true, "description": "URL of the document containing the CSS grouping." },
{ "name": "range", "$ref": "SourceRange", "optional": true, "description": "@-rule's header text range in the enclosing stylesheet (if available). This is from the first non-whitespace character after the @ declarartion to the last non-whitespace character before an opening curly bracket or semicolon." }
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/Headers.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -653,8 +653,10 @@ set(WebCore_PRIVATE_FRAMEWORK_HEADERS
css/parser/CSSParser.h
css/parser/CSSParserContext.h
css/parser/CSSParserMode.h
css/parser/CSSParserSelector.h
css/parser/CSSParserToken.h
css/parser/CSSParserTokenRange.h
css/parser/CSSSelectorParser.h

css/query/GenericMediaQueryTypes.h
css/query/MediaQuery.h
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/WebCore.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -3279,7 +3279,7 @@
949C77091D6E498700C0DE4F /* CSSParserObserverWrapper.h in Headers */ = {isa = PBXBuildFile; fileRef = 949C77071D6E48ED00C0DE4F /* CSSParserObserverWrapper.h */; };
949C770B1D6E49ED00C0DE4F /* CSSParserObserver.h in Headers */ = {isa = PBXBuildFile; fileRef = 949C770A1D6E49C300C0DE4F /* CSSParserObserver.h */; };
94DE5C821D7F3A1400164F2A /* CSSAtRuleID.h in Headers */ = {isa = PBXBuildFile; fileRef = 94DE5C801D7F39D000164F2A /* CSSAtRuleID.h */; };
94DE5C8E1D80802700164F2A /* CSSSelectorParser.h in Headers */ = {isa = PBXBuildFile; fileRef = 94DE5C8C1D80801500164F2A /* CSSSelectorParser.h */; };
94DE5C8E1D80802700164F2A /* CSSSelectorParser.h in Headers */ = {isa = PBXBuildFile; fileRef = 94DE5C8C1D80801500164F2A /* CSSSelectorParser.h */; settings = {ATTRIBUTES = (Private, ); }; };
94DE5C921D83011D00164F2A /* CSSSupportsParser.h in Headers */ = {isa = PBXBuildFile; fileRef = 94DE5C901D8300CB00164F2A /* CSSSupportsParser.h */; };
94DE5C961D8301BD00164F2A /* CSSParserImpl.h in Headers */ = {isa = PBXBuildFile; fileRef = 94DE5C941D8301B000164F2A /* CSSParserImpl.h */; };
94E839511DFB2A0E007BC6A7 /* CSSNamespaceRule.h in Headers */ = {isa = PBXBuildFile; fileRef = 94E8394F1DFB2700007BC6A7 /* CSSNamespaceRule.h */; settings = {ATTRIBUTES = (Private, ); }; };
Expand Down
29 changes: 29 additions & 0 deletions Source/WebCore/css/CSSStyleRule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "CSSStyleRule.h"

#include "CSSParser.h"
#include "CSSRuleList.h"
#include "CSSStyleSheet.h"
#include "DeclaredStylePropertyMap.h"
#include "PropertySetCSSStyleDeclaration.h"
Expand All @@ -45,6 +46,7 @@ CSSStyleRule::CSSStyleRule(StyleRule& styleRule, CSSStyleSheet* parent)
: CSSRule(parent)
, m_styleRule(styleRule)
, m_styleMap(DeclaredStylePropertyMap::create(*this))
, m_childRuleCSSOMWrappers(styleRule.nestedRules().size())
{
}

Expand Down Expand Up @@ -152,4 +154,31 @@ void CSSStyleRule::reattach(StyleRuleBase& rule)
m_propertiesCSSOMWrapper->reattach(m_styleRule->mutableProperties());
}

unsigned CSSStyleRule::length() const
{
return m_styleRule->nestedRules().size();
}

CSSRule* CSSStyleRule::item(unsigned index) const
{
if (index >= length())
return nullptr;

ASSERT(m_childRuleCSSOMWrappers.size() == m_styleRule->nestedRules().size());

auto& rule = m_childRuleCSSOMWrappers[index];
if (!rule)
rule = m_styleRule->nestedRules()[index]->createCSSOMWrapper(const_cast<CSSStyleRule&>(*this));

return rule.get();
}

CSSRuleList& CSSStyleRule::cssRules() const
{
if (!m_ruleListCSSOMWrapper)
m_ruleListCSSOMWrapper = makeUnique<LiveCSSRuleList<CSSStyleRule>>(const_cast<CSSStyleRule&>(*this));

return *m_ruleListCSSOMWrapper;
}

} // namespace WebCore
8 changes: 8 additions & 0 deletions Source/WebCore/css/CSSStyleRule.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

namespace WebCore {

class CSSRuleList;
class CSSStyleDeclaration;
class DeclaredStylePropertyMap;
class StylePropertyMap;
Expand All @@ -46,6 +47,10 @@ class CSSStyleRule final : public CSSRule, public CanMakeWeakPtr<CSSStyleRule> {
// FIXME: Not CSSOM. Remove.
StyleRule& styleRule() const { return m_styleRule.get(); }

CSSRuleList& cssRules() const;
unsigned length() const;
CSSRule* item(unsigned index) const;

StylePropertyMap& styleMap();

private:
Expand All @@ -60,6 +65,9 @@ class CSSStyleRule final : public CSSRule, public CanMakeWeakPtr<CSSStyleRule> {
Ref<StyleRule> m_styleRule;
Ref<DeclaredStylePropertyMap> m_styleMap;
RefPtr<StyleRuleCSSStyleDeclaration> m_propertiesCSSOMWrapper;

mutable Vector<RefPtr<CSSRule>> m_childRuleCSSOMWrappers;
mutable std::unique_ptr<CSSRuleList> m_ruleListCSSOMWrapper;
};

} // namespace WebCore
Expand Down
Loading

0 comments on commit b5cf192

Please sign in to comment.