Bug 107441 – Web Inspector: [Network] Add cookie column to show presence of request/response cookies.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Eugene Klyuchnikov]

In some use cases only responses that contain "set-cookie" header are interesting.

We should add a control to filter out items that do not bring "set-cookie" headers.

[Eugene Klyuchnikov]

Created attachment 183763 [details]
Patch

[Andrey Adaikin]

Comment on attachment 183763 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183763&action=review

> Source/WebCore/inspector/front-end/NetworkPanel.js:2039
> +            element.addStyleClass("network-has-cookies-in-response");

element.enableStyleClass() ?

[Andrey Adaikin]

Comment on attachment 183763 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183763&action=review

> Source/WebCore/inspector/front-end/NetworkPanel.js:353
> +        filterBarElement.title = "Use Ctrl-click to select multiple types.";

WebInspector.UIString ?

[Eugene Klyuchnikov]

Comment on attachment 183763 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183763&action=review

>> Source/WebCore/inspector/front-end/NetworkPanel.js:353
>> +        filterBarElement.title = "Use Ctrl-click to select multiple types.";
> 
> WebInspector.UIString ?

Definitely. Done.

>> Source/WebCore/inspector/front-end/NetworkPanel.js:2039
>> +            element.addStyleClass("network-has-cookies-in-response");
> 
> element.enableStyleClass() ?

Fixed. Thanks.

[Eugene Klyuchnikov]

Created attachment 183891 [details]
Patch

[Pavel Feldman]

Comment on attachment 183891 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183891&action=review

I was thinking that if we had a new column (Cookies Set), that would be not visible by default, we would be able to sort by it.

> Source/WebCore/English.lproj/localizedStrings.js:437
> +localizedStrings["Use Ctrl-click to select multiple types."] = "Use Ctrl-click to select multiple types.";

It is probably going to Cmd+click on Mac.

[Eugene Klyuchnikov]

Created attachment 184724 [details]
Patch

[Eugene Klyuchnikov]

Comment on attachment 183891 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183891&action=review

>> Source/WebCore/English.lproj/localizedStrings.js:437
>> +localizedStrings["Use Ctrl-click to select multiple types."] = "Use Ctrl-click to select multiple types.";
> 
> It is probably going to Cmd+click on Mac.

Fixed.

[Andrey Adaikin]

Comment on attachment 184724 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184724&action=review

> Source/WebCore/inspector/front-end/NetworkPanel.js:2231
> +        this._cookiesCell.appendChild(document.createTextNode(value.join("")));

this._cookiesCell.createTextChild(value.join("")); ?

> Source/WebCore/inspector/front-end/NetworkPanel.js:2412
> +    if (aCookiesReceived < bCookiesReceived) {

comparing booleans? did you mean it?

[Pavel Feldman]

Comment on attachment 184724 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184724&action=review

> Source/WebCore/inspector/front-end/NetworkPanel.js:378
> +        filterBarElement.createChild("div", "scope-bar-divider");

Please extract this change.

> Source/WebCore/inspector/front-end/NetworkPanel.js:1124
> +                row.enableStyleClass("offscreen", !rowIsVisible);

ditto

[Eugene Klyuchnikov]

Comment on attachment 184724 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184724&action=review

>> Source/WebCore/inspector/front-end/NetworkPanel.js:378
>> +        filterBarElement.createChild("div", "scope-bar-divider");
> 
> Please extract this change.

Done

>> Source/WebCore/inspector/front-end/NetworkPanel.js:1124
>> +                row.enableStyleClass("offscreen", !rowIsVisible);
> 
> ditto

Fixed.

>> Source/WebCore/inspector/front-end/NetworkPanel.js:2231
>> +        this._cookiesCell.appendChild(document.createTextNode(value.join("")));
> 
> this._cookiesCell.createTextChild(value.join("")); ?

Done

>> Source/WebCore/inspector/front-end/NetworkPanel.js:2412
>> +    if (aCookiesReceived < bCookiesReceived) {
> 
> comparing booleans? did you mean it?

Surely. But well, I'll make it more clear.

[Eugene Klyuchnikov]

Created attachment 185184 [details]
Patch

[Pavel Feldman]

Comment on attachment 185184 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=185184&action=review

Please provide a screenshot. r- for unclear sorting.

> Source/WebCore/inspector/front-end/NetworkPanel.js:2228
> +        var responseCookies = this._request.responseCookies;

What about bidirectional arrow for the case or receive / send? Like U+21D0, U+21D2, U+21D4 (see http://en.wikipedia.org/wiki/Template:Unicode_chart_Arrows)

> Source/WebCore/inspector/front-end/NetworkPanel.js:2230
> +            value.push("\u25BC");

// Down triangle

> Source/WebCore/inspector/front-end/NetworkPanel.js:2235
> +            value.push("\u25B2");

// Up triangle

> Source/WebCore/inspector/front-end/NetworkPanel.js:2414
> +    var aScore = (a._request.responseCookies && a._request.responseCookies.length) ? 2 : 0;

I'd rather sort in the following manner: first sort by set-cookie, then sort by cookie, then the rest.

[Eugene Klyuchnikov]

Comment on attachment 185184 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=185184&action=review

>> Source/WebCore/inspector/front-end/NetworkPanel.js:2228
>> +        var responseCookies = this._request.responseCookies;
> 
> What about bidirectional arrow for the case or receive / send? Like U+21D0, U+21D2, U+21D4 (see http://en.wikipedia.org/wiki/Template:Unicode_chart_Arrows)

Double-arrows look ugly =(

>> Source/WebCore/inspector/front-end/NetworkPanel.js:2230
>> +            value.push("\u25BC");
> 
> // Down triangle

Done.

>> Source/WebCore/inspector/front-end/NetworkPanel.js:2235
>> +            value.push("\u25B2");
> 
> // Up triangle

Done.

>> Source/WebCore/inspector/front-end/NetworkPanel.js:2414
>> +    var aScore = (a._request.responseCookies && a._request.responseCookies.length) ? 2 : 0;
> 
> I'd rather sort in the following manner: first sort by set-cookie, then sort by cookie, then the rest.

This code does exactly as you said: first go set-cookie + cookie, then set-cookie alone, then cookie, and cookie free at the end.

[Eugene Klyuchnikov]

Created attachment 185221 [details]
Screenshot

[Eugene Klyuchnikov]

Created attachment 185222 [details]
Patch

[kov's GTK+ EWS bot]

Comment on attachment 185222 [details]
Patch

Attachment 185222 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/16183206

[Pavel Feldman]

> Double-arrows look ugly =(

Not as ugly as the screenshot attached to my taste!

> > 
> > I'd rather sort in the following manner: first sort by set-cookie, then sort by cookie, then the rest.
> 
> This code does exactly as you said: first go set-cookie + cookie, then set-cookie alone, then cookie, and cookie free at the end.

I don't see it sorting by the number of cookies.

[Andrey Adaikin]

Comment on attachment 185222 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=185222&action=review

> Source/WebCore/inspector/front-end/NetworkPanel.js:2417
> +    var bScore = (b._request.responseCookies && b._request.responseCookies.length) ? 2 : 0;

function score()
{
   ...
}
return score(b) - score(a); ?

[Eugene Klyuchnikov]

Created attachment 185757 [details]
Screenshot: two columns

[Eugene Klyuchnikov]

Created attachment 185762 [details]
Patch

[WebKit Review Bot]

Comment on attachment 185762 [details]
Patch

Clearing flags on attachment: 185762

Committed r141417: <http://trac.webkit.org/changeset/141417>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.