[Webkit-unassigned] [Bug 96733] Web Inspector: Display Named Flows in the Tabbed Pane of the "CSS Named Flows" Drawer

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Alexander Pavlov (apavlov) <apavlov at chromium.org>  2012-09-17 07:58:01 PST ---
View in context: https://bugs.webkit.org/attachment.cgi?id=164393&action=review

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:383
> +WebInspector.FlowTreeElement = function(flowContainer)

Nice

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:396
> +    get overset()

If possible, transform these into accessor and modifier methods (something like overset() and setOverset(newOverset)), since our latest decision is to transition from getters/setters to methods (we've had a lot of confusion about these, where sometimes they are methods, sometimes they are getters/setters, and accessing a property in a get-tish manner when its getter is actually a method results in a bug, which is not detectable by the compiler, alas). The code inside is fine.

> Source/WebCore/inspector/front-end/CSSNamedFlowView.js:222
> +                    this._updateRegionOverset(this._regionsTreeItem.children[regionsTreeChildIndex], newRegions[newRegionsIndex].regionOverset, oldRegions[oldRegionsIndex].regionOverset)

trailing semicolon lost

> Source/WebCore/inspector/front-end/CSSNamedFlowView.js:223
> +                ++oldRegionsIndex, ++newRegionsIndex, ++regionsTreeChildIndex;

We don't use this approach to update multiple variables - please make a separate statement for each of these, since this code is hardly readable for someone not familiar with what's happening here.

> Source/WebCore/inspector/front-end/CSSNamedFlowView.js:229
> +                ++newRegionsIndex, ++regionsTreeChildIndex;

Ditto

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