[Webkit-unassigned] [Bug 93090] Web Inspector: requests filtering in network tab

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=93090


Vsevolod Vlasov <vsevik at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #156897|review?                     |review-
               Flag|                            |




--- Comment #12 from Vsevolod Vlasov <vsevik at chromium.org>  2012-08-07 02:30:58 PST ---
(From update of attachment 156897)
View in context: https://bugs.webkit.org/attachment.cgi?id=156897&action=review

> Source/WebCore/inspector/front-end/FilterController.js:38
> +    this._filterControlElement = this._element.createChild("span", "toolbar-filter-control");

This element is redundant.

> Source/WebCore/inspector/front-end/FilterController.js:59
> +        this._performFilter('');

Please use double quotes.

> Source/WebCore/inspector/front-end/FilterController.js:60
> +        this._filterInputElement.value = '';

Please use double quotes.

> Source/WebCore/inspector/front-end/FilterController.js:63
> +    _performFilter: function(query) 

Compiler annotations please.

> Source/WebCore/inspector/front-end/FilterController.js:66
> +        if (typeof currentPanel.performFilter === 'function')

Please use double quotes.

> Source/WebCore/inspector/front-end/FilterController.js:78
> +            return false;

return;

> Source/WebCore/inspector/front-end/FilterController.js:81
> +        if (isEnterKey(event)) {

No need to handle enter since we perform filtering on input event anyway.

> Source/WebCore/inspector/front-end/FilterController.js:104
> +        var isMac = WebInspector.isMac();

isMac is used only once, please inline.

> Source/WebCore/inspector/front-end/FilterController.js:107
> +                var isFindKey = event.metaKey && !event.ctrlKey && !event.altKey && !event.shiftKey;

isFilterKey

> Source/WebCore/inspector/front-end/NetworkPanel.js:447
> +            this._filteredOutRequests.put(this._requests[i], true);

Just clear the map before the loop and fill it with non matching requests below.
Otherwise it looks confusing: you remove "hidden" style and mark request as filtered out at the same time.

> Source/WebCore/inspector/front-end/NetworkPanel.js:448
> +            if (!_match)

_match is never defined and is probably redundant.
Also wrong indent here and below.

> Source/WebCore/inspector/front-end/NetworkPanel.js:450
> +                node.element.addStyleClass("hidden");

I wouldn't use global hidden class here since it says nothing about the reason why the row is not visible. I would add .network-log-grid.data-grid tr.filtered-out instead

> Source/WebCore/inspector/front-end/NetworkPanel.js:1174
> +        this.performFilter('');

Please use double quotes.

> Source/WebCore/inspector/front-end/NetworkPanel.js:1181
> +        this._filterByContent(query);

What is the difference between performFilter and filterByContent? There is no need to extract one from another.

> Source/WebCore/inspector/front-end/NetworkPanel.js:1450
> +    performFilter: function(query){

Compiler annotations please.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.