[webkit-reviews] review requested: [Bug 16531] Web Inspector: Show cookie data in the request headers in network pane : [Attachment 71772] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 25 11:21:02 PDT 2010


Andrey Kosyakov <caseq at chromium.org> has asked	for review:
Bug 16531: Web Inspector: Show cookie data in the request headers in network
pane
https://bugs.webkit.org/show_bug.cgi?id=16531

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

------- Additional Comments from Andrey Kosyakov <caseq at chromium.org>
See https://bugs.webkit.org/show_bug.cgi?id=48226 for previous discussion of
the patch.

> > WebCore/WebCore.gypi:4424
> > +		 'inspector/front-end/ResourceCookiesView.js',
> 
> This is a tab, not a view.

Renamed to ResourceCookiesTab and moved it to ResourceView.js

> > WebCore/inspector/front-end/CookieItemsViewBase.js:33
> > +WebInspector.CookieItemsViewBase = function()
> 
> This could simply reside in CookieItemsView in form of helper methods /
component.

Moving this to CookieItemsView.js, but leaving everything else intact (we do
use a bit of polymorphism there).

> > WebCore/inspector/front-end/CookieParser.js:57
> > +		     this._addCookie(kv, "request");
> 
> Could you use constants for these?

Done.

> > 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".

Quite so, and similar statements hold true for all other possible values of
maxDescentLevel ;-)
Changed the base for the level so that 0 is the old behavior (top level only)
and 1 is one level below it, etc.

> > WebCore/inspector/front-end/DataGrid.js:404
> > +	     if (!this._columnWidthsInitialized &&
this.element.enclosingNodeOrSelfWithNodeName("body")) {
> 
> You could also check this.element.offsetWidth for === 0.

Done.


More information about the webkit-reviews mailing list