[webkit-reviews] review denied: [Bug 74292] Web Inspector: [Styles] Update selected DOM element styles whenever applicable media queries change : [Attachment 119208] 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 74292: Web Inspector: [Styles] Update selected DOM element styles whenever
applicable media queries change
https://bugs.webkit.org/show_bug.cgi?id=74292

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

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


> Source/WebCore/inspector/CodeGeneratorInspector.py:207
> +    skip_js_bind_domains = set(["Runtime", "DOMDebugger"])

I think we should unskip everything. Why does it even exists?

> Source/WebCore/inspector/Inspector.json:1749
> +		   "description": "Fires whenever a MediaQuery result changes
(the current implementation considers only viewport-dependent media features.)"


This is not clear to me. Could you elaborate more on when the event is actually
fired?

> Source/WebCore/inspector/InspectorCSSAgent.cpp:237
> +	   m_frontend->mediaQueryResultChanged();

Now that CSS domain emits events, we should make sure that it only does so when
the agent is enabled. You need to introduce enable/disable methods pair as in
the other domains and only generate messages while enabled. Actually, you might
want to set CSS agent to the instrumentingAgents set in the ::enable.

> Source/WebCore/inspector/front-end/CSSStyleModel.js:859
> +WebInspector.CSSDispatcher = function(cssModel)

Please jsdoc this new class.