Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web Inspector: Show parent style rules for nested style rules in Styles sidebar #8202

Conversation

patrickangle
Copy link
Contributor

@patrickangle patrickangle commented Jan 4, 2023

b5cf192

Web Inspector: Show parent style rules for nested style rules in Styles 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

e34f118

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe   πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk βœ… πŸ›  wincairo
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2   πŸ§ͺ api-mac βœ… πŸ§ͺ gtk-wk2
  πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1   πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ jsc βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  jsc-armv7
βœ… πŸ›  πŸ§ͺ jsc-arm64 βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ§ͺ jsc-armv7-tests
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ›  jsc-mips
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim βœ… πŸ§ͺ jsc-mips-tests

@patrickangle patrickangle self-assigned this Jan 4, 2023
@patrickangle patrickangle added the Web Inspector Bugs related to the WebKit Web Inspector. label Jan 4, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 4, 2023
@patrickangle patrickangle removed the merging-blocked Applied to prevent a change from being merged label Jan 4, 2023
@patrickangle patrickangle force-pushed the eng/b250088-inspector-css-nesting-support branch from e8a25f4 to e002df3 Compare January 4, 2023 20:17
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 4, 2023
@patrickangle patrickangle removed the merging-blocked Applied to prevent a change from being merged label Jan 4, 2023
@patrickangle patrickangle force-pushed the eng/b250088-inspector-css-nesting-support branch from e002df3 to aa8dcdb Compare January 4, 2023 21:54
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 4, 2023
@patrickangle patrickangle removed the merging-blocked Applied to prevent a change from being merged label Jan 4, 2023
@patrickangle patrickangle force-pushed the eng/b250088-inspector-css-nesting-support branch from aa8dcdb to fa13973 Compare January 4, 2023 23:13
@patrickangle patrickangle force-pushed the eng/b250088-inspector-css-nesting-support branch from fa13973 to 7f323f7 Compare January 4, 2023 23:14
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 5, 2023
@patrickangle patrickangle removed the merging-blocked Applied to prevent a change from being merged label Jan 5, 2023
@patrickangle patrickangle force-pushed the eng/b250088-inspector-css-nesting-support branch from 7f323f7 to d2f7cb5 Compare January 5, 2023 05:36
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 5, 2023
Copy link
Contributor

@rcaliman-apple rcaliman-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The front-end parts build elegantly on top of your prior work. Good job πŸ‘

Still pondering how better to display the parent rules to communicate what they are, but this work stands on its own.

Found a couple of other issues with groupings, but I'll open separate bugs:

  • tab navigation forwards from a previous rule skips the next one's groupings; backwards navigation works fine.
    Update: logged Bug 250134

  • it would be nice to include the grouping prefix when filtering in the Styles panel
    Filtering for the string media won't match @media (width: 1200px), but filtering for anything in the condition text will.
    Update: logged Bug 250136

@patrickangle patrickangle removed the merging-blocked Applied to prevent a change from being merged label Jan 5, 2023
@patrickangle patrickangle force-pushed the eng/b250088-inspector-css-nesting-support branch from d2f7cb5 to 97671b7 Compare January 5, 2023 20:24
@patrickangle patrickangle force-pushed the eng/b250088-inspector-css-nesting-support branch from 97671b7 to 5685ee3 Compare January 5, 2023 21:35
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 6, 2023
@patrickangle patrickangle removed the merging-blocked Applied to prevent a change from being merged label Jan 6, 2023
@patrickangle patrickangle force-pushed the eng/b250088-inspector-css-nesting-support branch from 5685ee3 to 9441639 Compare January 6, 2023 00:53
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 6, 2023
@patrickangle patrickangle removed the merging-blocked Applied to prevent a change from being merged label Jan 6, 2023
@patrickangle patrickangle force-pushed the eng/b250088-inspector-css-nesting-support branch from 9441639 to 5812776 Compare January 6, 2023 02:17
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 6, 2023
@patrickangle patrickangle removed the merging-blocked Applied to prevent a change from being merged label Jan 6, 2023
@patrickangle patrickangle force-pushed the eng/b250088-inspector-css-nesting-support branch from 5812776 to e34f118 Compare January 6, 2023 16:18
@patrickangle patrickangle added the merge-queue Applied to send a pull request to merge-queue label Jan 6, 2023
…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
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/b250088-inspector-css-nesting-support branch from e34f118 to b5cf192 Compare January 6, 2023 20:30
@webkit-commit-queue
Copy link
Collaborator

Committed 258555@main (b5cf192): https://commits.webkit.org/258555@main

Reviewed commits have been landed. Closing PR #8202 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit b5cf192 into WebKit:main Jan 6, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Web Inspector Bugs related to the WebKit Web Inspector.
Projects
None yet
7 participants