[webkit-reviews] review denied: [Bug 45662] Web Inspector: [Resources panel] Should be easy to copy data. : [Attachment 89893] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Annie Sullivan
<sullivan at chromium.org>'s request for review:
Bug 45662: Web Inspector: [Resources panel] Should be easy to copy data.
https://bugs.webkit.org/show_bug.cgi?id=45662

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

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

Overall looks good. You need an entry in localizedStrings.js as Yury suggests
to land this.

> Source/WebCore/inspector/front-end/NetworkPanel.js:1023
>	   InspectorFrontendHost.copyText(JSON.stringify(har));

It used to export values, now it copies them, so the name of the method is
misleading. Do not reuse it.

> Source/WebCore/inspector/front-end/NetworkPanel.js:1026
> +    _exportLocation: function(resource)

I.e. should be _copyLocation, etc.

> Source/WebCore/inspector/front-end/Resource.js:429
> +	       this._requestHeadersText += key + ": " +
this.requestHeaders[key] + '\n'; 

Please use double quotes.

> Source/WebCore/inspector/front-end/Resource.js:510
> +	       this._responseHeadersText += key + ": " +
this.responseHeaders[key] + '\n'; 

ditto.