[webkit-reviews] review denied: [Bug 93090] Web Inspector: requests filtering in network tab : [Attachment 158360] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 14 23:50:52 PDT 2012


Vsevolod Vlasov <vsevik at chromium.org> has denied Pavel Chadnov
<chadnov at google.com>'s request for review:
Bug 93090: Web Inspector: requests filtering in network tab
https://bugs.webkit.org/show_bug.cgi?id=93090

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

------- Additional Comments from Vsevolod Vlasov <vsevik at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=158360&action=review


> Source/WebCore/ChangeLog:8
> +	   New feature: there are both search bar and filter bar in network
tabw. One can swich them using checkbox "filtering".

Please fix typos.

> Source/WebCore/inspector/front-end/SearchController.js:46
> +    this._firstRowElement.createChild("td");

Why is this needed?

> Source/WebCore/inspector/front-end/SearchController.js:57
> +  

Extra blank line.

> Source/WebCore/inspector/front-end/SearchController.js:74
> +    this._filterInputElement.addEventListener("keydown",
this._onKeyDown.bind(this), true);

keydown handler has some search specific logic that should be treated
carefully.

> Source/WebCore/inspector/front-end/SearchController.js:79
> +    this._secondRowElement.createChild("td");

Why is this needed?

> Source/WebCore/inspector/front-end/SearchController.js:125
> +  

Extra blank line.

> Source/WebCore/inspector/front-end/SearchController.js:136
> +    this._searchOrFilterQuery = "";

unused

> Source/WebCore/inspector/front-end/SearchController.js:255
> +	   this._performSearch(this._searchInputElement.value);

Please remove this line, we don't want to change search behavior.

> Source/WebCore/inspector/front-end/SearchController.js:432
> +	       this._performSearch(this._filterInputElement.value, false,
false);

Please use this._searchInputElement for consistency.


More information about the webkit-reviews mailing list