[webkit-reviews] review granted: [Bug 200419] Web Inspector: Styles: show @supports CSS groupings : [Attachment 375494] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 5 19:42:37 PDT 2019

Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 200419: Web Inspector: Styles: show @supports CSS groupings

Attachment 375494: Patch


--- Comment #3 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 375494
  --> https://bugs.webkit.org/attachment.cgi?id=375494

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

r=me! Nice!

> Source/JavaScriptCore/inspector/protocol/CSS.json:224
> -	       "id": "CSSMedia",
> +	       "id": "Grouping",

I'm not sure `Grouping` is clear but I'm warming up to it.

I considered `AtRuleQuery`. Since these primarily match a media query or
feature query features, with <link media="..."> / <style media="..."> special

I suspect other at-rule features like `@page {...}`, `@font-face {...}` and
`@keyframes {...}` will require their own types.

> Source/JavaScriptCore/inspector/protocol/CSS.json:226
> +	       "description": "CSS @media/@supports descriptor.",

This says "@media/@supports" but has a longer list of supported things (@media,
@supports, @import, and synthesized from <link media="..."> / <style

> Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js:458
> +	   let groupings = this.groupings.filter((grouping) => grouping.text
!== "all");

Does the frontend filter out media queries of `all`? Maybe then the backend
shouldn't filter them itself?

> +	       this._groupingElements = groupings.map((grouping, i) => {

Nit: This doesn't make use of `i`

> +		   if (groupingTypeElement.children.length)
> +		       groupingTypeElement.append(", ");

This just blew my mind. I did not know that `d = document.createElement("div");
d.textContent = "foo"; d.children.length` is zero. I guess `children` is not

Maybe using `groupingTypeElement.childElementCount` might be better? I'd
probably have carried a boolean around to say whether or not we're appending
more to the currentGroupingType.

More information about the webkit-reviews mailing list