[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