[webkit-reviews] review denied: [Bug 96999] Web Inspector: [Styles] For group selectors, transmit their segments with the "matches" flag : [Attachment 164870] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 96999: Web Inspector: [Styles] For group selectors, transmit their segments
with the "matches" flag
https://bugs.webkit.org/show_bug.cgi?id=96999

Attachment 164870: Patch
https://bugs.webkit.org/attachment.cgi?id=164870&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=164870&action=review


> Source/WebCore/ChangeLog:11
> +	   * inspector/Inspector.json: Introduce SelectorSegment type and add
elementMatchData field to CSSRule.

Lets add a test!

> Source/WebCore/inspector/Inspector.json:2027
> +		   "id": "SelectorSegment",

SelectorList and Selector ?

> Source/WebCore/inspector/Inspector.json:2031
> +		       { "name": "matched", "type": "boolean", "optional":
true, "description": "Whether the related element matches this selector segment
(default: false)." }

matching ?

> Source/WebCore/inspector/Inspector.json:2073
> +		       { "name": "elementMatchData", "type": "array", "items":
{ "$ref": "SelectorSegment" }, "optional": true, "description": "Present for
style rules' grouping selectors if a context element is known and not all
individual selectors are matched. Contains element matching data for each
selector segment."},

I would introduce the SelectorList class and refer to it from here (it would
encapsulate selectorText and selectorRange)

> Source/WebCore/inspector/InspectorCSSAgent.cpp:684
> +    if (selectorList.hasOneSelector())

Lets provide this information at all times as a start - it would make front-end
life easier.