[webkit-reviews] review denied: [Bug 48226] Web Inspector: display cookies associated with request : [Attachment 71760] patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Andrey Kosyakov
<caseq at chromium.org>'s request for review:
Bug 48226: Web Inspector: display cookies associated with request
https://bugs.webkit.org/show_bug.cgi?id=48226

Attachment 71760: patch
https://bugs.webkit.org/attachment.cgi?id=71760&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=71760&action=review

> WebCore/ChangeLog:64
> +2010-10-25  Andrey Kosyakov	<caseq at chromium.org>

Two changelogs.

> WebCore/WebCore.gypi:4424
> +	       'inspector/front-end/ResourceCookiesView.js',

This is a tab, not a view.

> WebCore/inspector/front-end/CookieItemsViewBase.js:33
> +WebInspector.CookieItemsViewBase = function()

This could simply reside in CookieItemsView in form of helper methods /
component.

> WebCore/inspector/front-end/CookieParser.js:57
> +		   this._addCookie(kv, "request");

Could you use constants for these?

> WebCore/inspector/front-end/DataGrid.js:320
> +	   var children = maxDescentLevel ? this._enumerateChildren(this, [],
maxDescentLevel) : this.children;

So if I pass "1" as maxDescentLevel, the code will work as if it was "1".

> WebCore/inspector/front-end/DataGrid.js:404
> +	   if (!this._columnWidthsInitialized &&
this.element.enclosingNodeOrSelfWithNodeName("body")) {

You could also check this.element.offsetWidth for === 0.