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

[bugzilla-daemon at webkit.org]

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


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

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




--- Comment #18 from Alexander Pavlov (apavlov) <apavlov at chromium.org>  2012-09-13 08:11:32 PST ---
(From update of attachment 163837)
View in context: https://bugs.webkit.org/attachment.cgi?id=163837&action=review

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:68
> +    appendApplicableItems: function(contextMenu, target)

Please annotate the methods that have arguments and/or return values. This will immensely simplify the code maintenance and improve code quality. Sorry I didn't mention this in the original review.

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:249
> +    _hashNamedFlow: function(namedFlow, eventData)

Ugh, this is definitely not what I meant... I didn't notice that the methods had different argument types. But to keep things simple, you can provide documentNodeId and flowName as separate arguments to this method (since these are the only things that are actually needed.) The calls will be, respectively,

this._hashNamedFlow(flow.documentNodeId, flow.name);
and
this._hashNamedFlow(event.data.documentNodeId, event.data.flowName);

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

While we are here, does the "flow" prefix do anything useful? Are you perhaps going to store some other kind of hash in the same "map"?

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