I'm missing the cookie data that's being sent to the server in the request information when you click on an item in the Network section inside web inspector. (I have to debug a problem there so that's why I am missing that information).
<rdar://problem/5712895>
Now that we have a Cookie storage view, is this enough to have here, or should there be more?
I think showing them with resources might be nice, but can be confusing too. We should only show the cookies that were sent to / set by the resource.
Do we have that data from CFNetwork? I don't think we do, we can get all the cookies that the resource has access to, but not the ones just set by that resource.
We should have the Cookies header that they send back.
(In reply to comment #2) > Now that we have a Cookie storage view, is this enough to have here, or should there be more? I was told that my post on the Chromium group relates to this issue. http://groups.google.com/a/chromium.org/group/chromium-discuss/browse_thread/thread/c44361ebe6e03184# Personally, I think that showing cookie information in the Resource tab is very important, and that it shouldn't only be available in the Cookie storage view. I'm not familiar with WebKit besides using Chrome, but I'm assuming this is referring to the "Cookies and Other Data" dialog. I don't understand why cookies would not be included with the Request Headers and Response Headers in Developer Tools so that developers can get a very clear picture of everything related to a request. When debugging a cookie related problem, it is extremely cumbersome to have to go into a separate location to view cookie details. Having an indicator showing when a new cookie is set would be nice (perhaps bold it or something). Also, the separate cookie storage view simply shows the state of cookies at a certain point in time - when you look at it. It doesn't give you an easy way to see which request received a cookie, or which ones sent a cookie. For instance, if there are a series of ajax requests, one of which receives a cookie, and the following ones send the cookie, it would be hard to determine which ones used cookies and which didn't.
*** Bug 48226 has been marked as a duplicate of this bug. ***
Created attachment 71772 [details] patch 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.
Comment on attachment 71772 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=71772&action=review > WebCore/inspector/front-end/CookieItemsView.js:30 > +WebInspector.CookieItemsViewBase = function() Can we call this CookiesTable?
Manually committed r70529: http://trac.webkit.org/changeset/70529 Note this functionality depends on platform's ability to deliver Cookie/Set-Cookie headers from the network stack, which is not yet available on many platforms yet -- so this patch won't have immediate effect on these platforms. Chromium, for one, has this working.
Seems this is fixed. Please consider using webkit-patch land, as it will close bugs for you when landing.
I left it open on purpose (see comment #10), as this was only fixed for Chromium -- this functionality depends on the network stack supplying actual request headers, which is only implemented in case of Chromium presently. Hence, the cookies tab is currently enabled conditionally (off by default). I thought it wouldn't be appropriate to close this bug until it gets fixed in other browsers -- Safari in particular, given Radar references above.
Attachment 71772 [details] was posted by a committer and has review+, assigning to Andrey Kosyakov for commit.
Comment on attachment 71772 [details] patch Marking obsolete since this was landed.
This is ticket is marked REOPENED, but should be marked CLOSED/FIXED. See: Comment #14 From Eric Seidel 2010-12-20 22:59:48 PST
This information is now available to WebKit on macOS via CFNetwork's public didFinishCollectingMetrics delegate message. WebKit has access to a complete NSURLRequest which includes all of the request headers that were sent, including Cookie headers, Authorization headers (Basic Auth), and others (Host, Accept*) which may be added underneath WebKit.
Created attachment 306241 [details] [IMAGE] Before - Missing Cookie Header
Created attachment 306242 [details] [IMAGE] After - Include Cookie, Accept-Language, Accept-Encoding
Created attachment 306244 [details] [PATCH] Proposed Fix Some things I have to test: 1. Is it possible to send a request with duplicate header? if so I need a test for that 2. I have concerns about potential performance issues: - this makes a HTTPHeaderMap for every request and serializes between the NetworkProcess and WebProcess (by putting it in NetworkLoadMetrics) - I suspect this will be normally be small but it could be large and if so I want to make sure we don't negatively impact load performance
Comment on attachment 306244 [details] [PATCH] Proposed Fix Awesome!
(In reply to Joseph Pecoraro from comment #19) > Created attachment 306244 [details] > [PATCH] Proposed Fix > > Some things I have to test: > > 1. Is it possible to send a request with duplicate header? if so I need a > test for that Except for cookie headers, duplicated headers should be combined. > 2. I have concerns about potential performance issues: > - this makes a HTTPHeaderMap for every request and serializes between the > NetworkProcess and WebProcess (by putting it in NetworkLoadMetrics) > - I suspect this will be normally be small but it could be large and if so > I want to make sure we don't negatively impact load performance This will happen in all cases or just when web inspector is set?
Comment on attachment 306244 [details] [PATCH] Proposed Fix Attachment 306244 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3475445 New failing tests: http/tests/inspector/network/resource-request-headers.html
Created attachment 306249 [details] Archive of layout-test-results from ews107 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Oh, el-capitan doesn't use the NSURLSession path, so I'll have to mark this as failing there like I did for the WK1 ports.
(In reply to youenn fablet from comment #21) > (In reply to Joseph Pecoraro from comment #19) > > Created attachment 306244 [details] > > [PATCH] Proposed Fix > > > > Some things I have to test: > > > > 1. Is it possible to send a request with duplicate header? if so I need a > > test for that > > Except for cookie headers, duplicated headers should be combined. Yeah, I don't see a way for web developers to ever create a case of duplicate request headers (XHR and Fetch both have set/append so only 1 key). I may have missed a case where the browser might do it, but in that case I concluded it would be enough of an edge case that I can worry about it later. > > 2. I have concerns about potential performance issues: > > - this makes a HTTPHeaderMap for every request and serializes between the > > NetworkProcess and WebProcess (by putting it in NetworkLoadMetrics) > > - I suspect this will be normally be small but it could be large and if so > > I want to make sure we don't negatively impact load performance > > This will happen in all cases or just when web inspector is set? Bug 170525 convert only doing this when Web Inspector is open. If that gets reviewed positively I'll land this confidently (I don't want to impact performance of general browsing).
<https://trac.webkit.org/changeset/215062/webkit>