[Webkit-unassigned] [Bug 107813] Web Inspector: Filters on Console panel

[bugzilla-daemon at webkit.org]

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


Pavel Feldman <pfeldman at chromium.org> changed:

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




--- Comment #10 from Pavel Feldman <pfeldman at chromium.org>  2013-01-28 09:36:21 PST ---
(From update of attachment 184996)
View in context: https://bugs.webkit.org/attachment.cgi?id=184996&action=review

> Source/WebCore/inspector/front-end/ConsoleView.js:491
> +    _addMessageUrlFilter: function(url)

nit: URL

> Source/WebCore/inspector/front-end/ConsoleView.js:493
> +        this._messageURLFilters.push(url);

nit: as a next change, you'd want to persist this.

> Source/WebCore/inspector/front-end/ConsoleView.js:501
> +    _removeMessageUrlFilter: function(index)

ditto

> Source/WebCore/inspector/front-end/ConsoleView.js:518
> +        return !message.url || this._messageURLFilters.indexOf(message.url) === -1;

This is a linear search which we don't need to do: making filters an object (map from string to boolean) would speed things up.

> Source/WebCore/inspector/front-end/ConsoleView.js:812
> +            this.messagesElement.insertBefore(element, node);

no need for {} around single line block

> LayoutTests/inspector/console/console-filter-test.html:3
> +    <script src="../../http/tests/inspector/inspector-test.js"></script>

nit: we don't indent these - see other tests.

> LayoutTests/inspector/console/console-filter-test.html:7
> +        function log1() {

{ on the next line.

> LayoutTests/inspector/console/console-filter-test.html:12
> +        function onload() {

ditto

> LayoutTests/inspector/console/console-filter-test.html:13
> +            for (var i = 0; i < 10; i++)

missing {}

> LayoutTests/inspector/console/console-filter-test.html:24
> +        function test() {

no indent for top level functions. { on the next line.

> LayoutTests/inspector/console/console-filter-test.html:36
> +            InspectorTest.addResult("=== BEFORE FILTER ===");

You should use runTestSuite to make several tests handling different cases (at least incremental + update)

> LayoutTests/inspector/console/resources/log-source.js:1
> +function log2() {

{ on the next line.

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