[Webkit-unassigned] [Bug 90871] Web Inspector: Display Named Flows in the "CSS Named Flows" drawer

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 13 02:47:17 PDT 2012


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


Alexander Pavlov (apavlov) <apavlov at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #163805|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #16 from Alexander Pavlov (apavlov) <apavlov at chromium.org>  2012-09-13 02:47:41 PST ---
(From update of attachment 163805)
View in context: https://bugs.webkit.org/attachment.cgi?id=163805&action=review

You may want to somehow remove the tEXt ancillary chunks containing "Creation Date" and "Software" from namedFlowOverflow.png in order to reduce its size.

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:1
> +/**

License boilerplate header missing

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:72
> +    _sidebarHasContent: function()

This naming convention (same for _sidebarIsEmpty) implies that the methods are accessors return a boolean. You should use something like "_setSidebarHasContent(boolean)" instead.

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:93
> +        var flowContainer = {

No need to expand this onto multiple lines, we usually put this onto a single line.

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:96
> +        }

Trailing ';' missing

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:194
> +        // FIXME: we only have support for Named Flows in the main document

Even though it is quite arbitrary in the legacy code, the text following FIXME: should be a full sentence (capitalized, with a trailing period), and we try to stick to this convention in the new code. Ditto for the following methods.

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:204
> +        if (event.data.documentNodeId !== this._document.id)

In order to avoid compilation errors, declare
var data = /** @type {WebInspector.NamedFlow} */ event.data;
and then handle the |data| var instead.

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:219
> +    hashNamedFlow: function(namedFlow)

All of these should be private methods (with a leading underscore)

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:221
> +        return "flow" + namedFlow.documentNodeId + namedFlow.name;

Can a flow name start with a digit? If so, we have a potential collision here.

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:224
> +    hashRemovedNamedFlow: function(eventData)

You don't need this method (use hashNamedFlow instead).

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:244
> +    showNamedFlowForNode: function(node)

Should be private

> Source/WebCore/inspector/front-end/cssNamedFlows.css:1
> +.css-named-flow-collections-view .split-view-sidebar-left {

License boilerplate header missing

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



More information about the webkit-unassigned mailing list