[webkit-reviews] review granted: [Bug 66381] Web Inspector: support multiple selection/deletion of cookie records : [Attachment 353635] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Matt Baker <mattbaker at apple.com>'s
request for review:
Bug 66381: Web Inspector: support multiple selection/deletion of cookie records
https://bugs.webkit.org/show_bug.cgi?id=66381

Attachment 353635: Patch

https://bugs.webkit.org/attachment.cgi?id=353635&action=review




--- Comment #19 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 353635
  --> https://bugs.webkit.org/attachment.cgi?id=353635
Patch

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

r=me, nice work!

> Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js:110
> +	       let cookieURL = (cookie.secure ? "https://" : "http://") +
cookie.domain + cookie.path;

This should probably be a member of `WI.Cookie`.

> Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js:164
> +	       minWidth: 70,

Just out of curiosity, where did you get these values?

> Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js:317
> +    _reloadCookies()

It might be worth adding this to a `shown()` function, so that it auto-updates
whenever the view becomes visible (only once `didInitialLayout`).

> Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js:319
> +	   PageAgent.getCookies().then((payload) => {

I wish we had a manager for this so that it isn't directly invoked by view code
:(