Skip to content
Permalink
Browse files
Web Inspector: Support editing @rules in the Styles sidebar
https://bugs.webkit.org/show_bug.cgi?id=246768
rdar://99871652

Reviewed by Devin Rousso.

This work is best thought of in three interlocking pieces:
- Rework chunks of InspectorStyleSheet to allow us to determine source data for @rules, including the header range,
which is the range of the text between `@media` (and friends) and the opening curly bracket for the rule set.
- Generalizing setting a rule's selector to just be setting its header text so we can do so with non-style rules.
- Implement frontend support for editing these rules in the same way selectors are currently editable.

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

* LayoutTests/inspector/css/generateFormattedText-expected.txt:
* LayoutTests/inspector/css/getMatchedStylesForNode-expected.txt:
- Rebaseline expectations for new CSS.Grouping properties.

* Source/JavaScriptCore/inspector/protocol/CSS.json:
- Add rule ID to CSS.Grouping so that we can use it later to edit the header text of the grouping.
- Add method CSS.setGroupingHeaderText to change a CSS.Grouping's header text.

* Source/WebCore/css/parser/CSSParser.h:
* Source/WebCore/css/parser/CSSParserImpl.cpp:
(WebCore::CSSParserImpl::consumeContainerRule):
- Provide a correct header range by keeping a copy of the original prelude before consumption to use for informing the
observer.

* Source/WebCore/css/parser/CSSParserImpl.h:
- Expose `consumeAtRule` publicly for use in InspectorStyleSheet

* Source/WebCore/inspector/InspectorStyleSheet.cpp:
(WebCore::flatteningStrategyForStyleRuleType):
- Move the logic for determining how we unflatten rules to a shared static method so that the source data and flat CSSOM
rule lists always consider the same set of rules in the same way.

(WebCore::parserContextForDocument):
(WebCore::isValidRuleHeaderText):
(WebCore::protocolGroupingTypeForStyleRuleType):
- Abstract logic that relies on rule type into static methods so that the various bits that need implemented when adding
an editable rule type are in close proximity to each other.

(WebCore::flattenSourceData):
(WebCore::ParsedStyleSheet::setSourceData):
- Use `flatteningStrategyForStyleRuleType` to determine how we should handle each piece of source data.

(WebCore::StyleSheetHandler::startRuleHeader):
(WebCore::StyleSheetHandler::setRuleHeaderEnd):
- Fix to never produce a range end that occurs before the range's start.

(WebCore::sourceURLForCSSRule):
(WebCore::InspectorStyleSheet::buildObjectForGrouping):
(WebCore::InspectorStyleSheet::buildArrayForGroupings):
- Split building the grouping array into a few discrete, reusable pieces. For the four types of rules we are making
editable, we will use `buildObjectForGrouping` to build those rule objects. For other real that are currently turned
into fake groupings, we leave the logic mostly intact (which means they are not editable).

(WebCore::InspectorStyleSheet::ruleHeaderText):
(WebCore::InspectorStyleSheet::setRuleHeaderText):
(WebCore::InspectorStyleSheet::ruleSelector): Deleted.
(WebCore::isValidSelectorListString): Deleted.
(WebCore::InspectorStyleSheet::setRuleSelector): Deleted.
- Setting a rule's header text and setting a rule's selector(s) are (except for some optimizations) the same operation.
For CSSStyleRule we still follow the existing logic of modifying a selector via CSSOM, which saves us from reparsing the
entire style sheet. For other rules, this is more complicated because our CSSOM implementation doesn't allow mutating
the header text, and changing these rules could change which rules apply to which nodes beyond just the rule we are
mutating.

(WebCore::InspectorStyleSheet::deleteRule):
(WebCore::InspectorStyleSheet::ruleForId const):
(WebCore::InspectorStyleSheet::cssStyleRulesSplitFromSameRule):
(WebCore::InspectorStyleSheet::buildObjectForSelectorList):
(WebCore::InspectorStyleSheet::buildObjectForRule):
(WebCore::InspectorStyleSheet::styleForId const):
(WebCore::InspectorStyleSheet::ruleOrStyleId const):
(WebCore::InspectorStyleSheet::ruleSourceDataFor const):
(WebCore::InspectorStyleSheet::ruleIndexByStyle const):
(WebCore::InspectorStyleSheet::buildArrayForRuleList):
(WebCore::InspectorStyleSheet::collectFlatRules):
(WebCore::InspectorStyleSheet::ruleId const): Deleted.
- Don't assume that a CSSRule will always be a CSSStyleRule.
- Allow both styles and rules to be used to look up the ID for a rule/style.

* Source/WebCore/inspector/InspectorStyleSheet.h:
(WebCore::InspectorStyleSheet::canBind const):
(WebCore::InspectorStyleSheet::styleId const): Deleted.

* Source/WebCore/inspector/agents/InspectorCSSAgent.cpp:
(WebCore::InspectorCSSAgent::setRuleSelector):
(WebCore::InspectorCSSAgent::setGroupingHeaderText):
* Source/WebCore/inspector/agents/InspectorCSSAgent.h:
- Generalize setting selectors to also support setting header text of @rules.

(WebCore::InspectorCSSAgent::addRule):
- Don't assume that all rules are CSSStyleRules.

(WebCore::InspectorCSSAgent::asCSSStyleRule): Deleted.
- Remove helper that was only used in a few places and is better served by a dynamicDowncast anyways.

* Source/WebInspectorUI/UserInterface/Main.html:

* Source/WebInspectorUI/UserInterface/Models/CSSGrouping.js:
(WI.CSSGrouping.prototype.get ownerStyleSheet):
(WI.CSSGrouping.prototype.get id):
(WI.CSSGrouping.prototype.get type):
(WI.CSSGrouping.prototype.get editable):
(WI.CSSGrouping.prototype.get text):
- Add necessary information for editing, which is mostly keeping track of our nodeStyles and ownerStyleSheet like style
rules do, and accepting an optional ID, which when present allows us to perform edits.

(WI.CSSGrouping.prototype.async setText):
- Handle setting the text for the grouping. For groupings that no longer apply after modification, we need to update the
current grouping object so that it is accurate to the current state, as it will remain visible to the user until they
switch to a different node.

* Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js:
(WI.DOMNodeStyles):
(WI.DOMNodeStyles.prototype.set ignoreNextContentDidChangeForStyleSheet):
(WI.DOMNodeStyles.prototype.refresh.fetchedMatchedStyles):
(WI.DOMNodeStyles.prototype._parseRulePayload):
- Parse the additional data in the payload to support editing.
- Use a shared representation of CSSGrouping's with an ID so that we can update the common representation during an
edit, thus allowing that same rule to update in all the locations it is visible for the current node. (It is not
necessary to update for other nodes, since they will have been invalidated by the stylesheet change and will reload
their styles.)

* Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.css:
(.spreadsheet-css-declaration :is(.selector, .grouping).spreadsheet-rule-header-field):
(.spreadsheet-css-declaration :is(.selector, .grouping).spreadsheet-rule-header-field.editing):
- Style the grouping editor field the same way we style the selector editor field.

* Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:
(WI.SpreadsheetCSSStyleDeclarationSection):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype.initialLayout):
- Remove logic that lays out the CSS groupings, since those are dynamic and may change on future updates.

(WI.SpreadsheetCSSStyleDeclarationSection.prototype.layout):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._layoutGroupings.this._groupingElements.groupings.map.):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._layoutGroupings):
- Update groupings when layout is called to get the latest information from DOMNodeStyles.
- No longer merge together adjacent @rules like multiple @media or @layer rules, as those rules are editable now, and we
need to accurately represent the author's rules.
- Associate a text field with the grouping header text when applicable to enable editing.

(WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetRuleHeaderFieldDidCommit):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetRuleHeaderFieldWillNavigate):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetRuleHeaderFieldDidDiscard):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetSelectorFieldDidCommit): Deleted.
(WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetSelectorFieldWillNavigate): Deleted.
(WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetSelectorFieldDidDiscard): Deleted.
- Share the delegate methods for the selector and the grouping fields.

(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleSpreadsheetSelectorFieldStartedEditing):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleSpreadsheetSelectorFieldStoppedEditing):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleSpreadsheetSelectorFieldDidCommit):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleSpreadsheetSelectorFieldWillNavigate):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleSpreadsheetGroupingFieldDidCommit):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleSpreadsheetGroupingFieldWillNavigate):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleSpreadsheetGroupingFieldDidDiscard):

* Source/WebInspectorUI/UserInterface/Views/SpreadsheetRuleHeaderField.js: Renamed from Source/WebInspectorUI/UserInterface/Views/SpreadsheetSelectorField.js.
(WI.SpreadsheetRuleHeaderField):
(WI.SpreadsheetRuleHeaderField.prototype.get element):
(WI.SpreadsheetRuleHeaderField.prototype.startEditing):
(WI.SpreadsheetRuleHeaderField.prototype.stopEditing):
(WI.SpreadsheetRuleHeaderField.prototype._handleBlur):
(WI.SpreadsheetRuleHeaderField.prototype._handleKeyDown):
- Delegate methods should provide the current text field as their first parameter to allow the delegate to determine the
appropriate action.

* Source/WebInspectorUI/UserInterface/Views/StyleOriginView.js:
(WI.StyleOriginView.prototype.update):
(WI.StyleOriginView):

Canonical link: https://commits.webkit.org/256043@main
  • Loading branch information
patrickangle committed Oct 27, 2022
1 parent 216acdc commit 33803c89a12929f179cf29e76afc037e6c5fdf85
Show file tree
Hide file tree
Showing 20 changed files with 1,056 additions and 301 deletions.
@@ -50,8 +50,8 @@ a: 1; b: 2; c: 3;
a: 1; b: 2; c: 3;

-- Running test case: CSS.generateFormattedText.MatchedRules.includeGroupingsAndSelectors
@media only screen and (min-width: 0px) {@media only screen and (min-height: 0px) {body > div#test-node {a: 1; b: 2; c: 3;}}}
@media only screen and (min-width: 0px) {body > #test-node {a: 1; b: 2; c: 3;}}
@media only screen and (min-width:0px) {@media only screen and (min-height:0px) {body > div#test-node {a: 1; b: 2; c: 3;}}}
@media only screen and (min-width:0px) {body > #test-node {a: 1; b: 2; c: 3;}}
body > div {a: 1; b: 2; c: 3;}
body > * {a: 1; b: 2; c: 3;}
* {a: 1; b: 2; c: 3;}
@@ -84,16 +84,16 @@ body > * {a: 1; b: 2; c: 3;}


-- Running test case: CSS.generateFormattedText.MatchedRules.includeGroupingsAndSelectors.Multiline
@media only screen and (min-width: 0px) {
@media only screen and (min-height: 0px) {
@media only screen and (min-width:0px) {
@media only screen and (min-height:0px) {
body > div#test-node {
a: 1;
b: 2;
c: 3;
}
}
}
@media only screen and (min-width: 0px) {
@media only screen and (min-width:0px) {
body > #test-node {
a: 1;
b: 2;
@@ -124,8 +124,8 @@ a: 1; /* b: 2; */ c: 3;
a: 1; /* b: 2; */ c: 3;

-- Running test case: CSS.generateFormattedText.MatchedRules.WithCommentedProperties.includeGroupingsAndSelectors
@media only screen and (min-width: 0px) {@media only screen and (min-height: 0px) {body > div#test-node {a: 1; /* b: 2; */ c: 3;}}}
@media only screen and (min-width: 0px) {body > #test-node {a: 1; /* b: 2; */ c: 3;}}
@media only screen and (min-width:0px) {@media only screen and (min-height:0px) {body > div#test-node {a: 1; /* b: 2; */ c: 3;}}}
@media only screen and (min-width:0px) {body > #test-node {a: 1; /* b: 2; */ c: 3;}}
body > div {a: 1; /* b: 2; */ c: 3;}
body > * {a: 1; /* b: 2; */ c: 3;}
* {a: 1; /* b: 2; */ c: 3;}
@@ -158,16 +158,16 @@ body > * {a: 1; /* b: 2; */ c: 3;}


-- Running test case: CSS.generateFormattedText.MatchedRules.WithCommentedProperties.includeGroupingsAndSelectors.Multiline
@media only screen and (min-width: 0px) {
@media only screen and (min-height: 0px) {
@media only screen and (min-width:0px) {
@media only screen and (min-height:0px) {
body > div#test-node {
a: 1;
/* b: 2; */
c: 3;
}
}
}
@media only screen and (min-width: 0px) {
@media only screen and (min-width:0px) {
body > #test-node {
a: 1;
/* b: 2; */
@@ -998,7 +998,9 @@ Inherited:
"groupings": [
{
"type": "media-rule",
"ruleId": "<filtered>",
"text": "(min-width: 1px)",
"range": "<filtered>",
"sourceURL": "<filtered>"
}
]
@@ -1048,12 +1050,16 @@ Inherited:
"groupings": [
{
"type": "supports-rule",
"ruleId": "<filtered>",
"text": "(display: block)",
"range": "<filtered>",
"sourceURL": "<filtered>"
},
{
"type": "media-rule",
"ruleId": "<filtered>",
"text": "(min-width: 2px)",
"range": "<filtered>",
"sourceURL": "<filtered>"
}
]
@@ -0,0 +1,188 @@
Tests for the CSS.setGroupingHeaderText command.


== Running test suite: CSS.setGroupingHeaderText
-- Running test case: CSS.setGroupingHeaderText.MediaRule
PASS: Should have 2 authored rule.
PASS: Should find grouping of type 'media-rule'.
Setting text to 'not print'.
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 '(max-width: 9999px)'.
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 'screen and (max-width: 9999px)'.
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 'screen, (max-width: 9999px)'.
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 'not all'.
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 '(totally-not-supported-but-valid-syntactivally: 42em)'.
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 'invalidkeyword, (max-width: 9999px)'.
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 '(max-width: 9999px), invalidkeyword'.
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 'screen {} @media screen'.
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 'screen'.
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.SupportsRule
PASS: Should have 2 authored rule.
PASS: Should find grouping of type 'supports-rule'.
Setting text to '(color: purple)'.
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 '(color: purple) and (color: green)'.
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 'selector(h2 > p)'.
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 'not (display: grid)'.
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 'not (not (display: grid))'.
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 'not (not (not (display: grid)))'.
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 '(totally-not-supported-but-valid-syntactivally: 42em)'.
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 'not all'.
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 '|| and (max-width: 9999px)'.
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 '(max-width: 9999px) and ||'.
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 '(color: red) {} @supports(color: red)'.
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 '(color: red)'.
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.LayerRule
PASS: Should have 2 authored rule.
PASS: Should find grouping of type 'layer-rule'.
Setting text to 'howdy'.
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 'hello.howdy'.
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 'hello, howdy'.
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 'hello howdy'.
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 'hello {} @layer hello'.
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 'hello'.
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.ContainerRule
PASS: Should have 2 authored rule.
PASS: Should find grouping of type 'container-rule'.
Setting text to 'name (min-width: 42px)'.
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 '(max-width: 9999px)'.
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 '(totally-not-supported-but-valid-syntactivally: 42em)'.
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 'screen and (max-width: 9999px)'.
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 'screen, (max-width: 9999px)'.
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 'not all'.
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 'invalidkeyword, (max-width: 9999px)'.
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 '(max-width: 9999px), invalidkeyword'.
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 '(min-width:1px) {} @container(min-width:1px)'.
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 '(min-width: 1px)'.
PASS: Setting text should succeed.
PASS: Text should change.
PASS: Text should change on the same grouping attached to a different rule.

0 comments on commit 33803c8

Please sign in to comment.