[webkit-reviews] review denied: [Bug 112710] Web Inspector: Remove remainings of CSS-based console message filtering. : [Attachment 193998] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Dmitry Zvorygin
<zvorygin at chromium.org>'s request for review:
Bug 112710: Web Inspector: Remove remainings of CSS-based console message
filtering.
https://bugs.webkit.org/show_bug.cgi?id=112710

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

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


> Source/WebCore/inspector/front-end/ConsoleView.js:-274
> -	       if (target.hasStyleClass("selected")) {

Why did this change?

> Source/WebCore/inspector/front-end/ConsoleView.js:-288
> -	   if (!selectMultiple) {

This seems to ruin the diff. Can you leave existing condition nesting?

> Source/WebCore/inspector/front-end/ConsoleView.js:539
> +	   return (!message.url || !this._messageURLFilters[message.url]) &&
(!message.level || this._messageLevelFilters.indexOf(message.level) != -1);

This might get a bit expensive - consider using an object for faster lookup.

> Source/WebCore/inspector/front-end/ConsoleView.js:774
> + * @param wasThrown

You should specify types for parameters and list them in proper order.