[webkit-reviews] review granted: [Bug 119774] Web Inspector: Download Web Archive of Inspected Page : [Attachment 208763] [PATCH] Proposed Fix - addressed review comments

[bugzilla-daemon at webkit.org]

Antoine Quint <graouts at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 119774: Web Inspector: Download Web Archive of Inspected Page
https://bugs.webkit.org/show_bug.cgi?id=119774

Attachment 208763: [PATCH] Proposed Fix - addressed review comments
https://bugs.webkit.org/attachment.cgi?id=208763&action=review

------- Additional Comments from Antoine Quint <graouts at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=208763&action=review


> Source/WebInspectorUI/UserInterface/DOMTreeContentView.js:158
> +    get supportsSave()
> +    {
> +	   var archiveable =
WebInspector.Resource.Type.fromMIMEType(WebInspector.frameResourceManager.mainF
rame.mainResource.mimeType) === WebInspector.Resource.Type.Document;
> +	   return archiveable;
> +    },

I would just return directly and not use the variable. Also, does it need to be
Public?

> Source/WebInspectorUI/UserInterface/DOMTreeContentView.js:444
> +    _downloadArchive: function(forceSaveAs)
> +    {
> +	   this._downloadArchiveNavigationItem.enabled = false;

Seems to me that while we wait for the callback we should turn the
_downloadArchiveNavigationItem into a spinner or something that shows the
action is in progress, since it could take a while on iOS. This would be fine
as a followup.

> Source/WebInspectorUI/UserInterface/InspectorBackendCommands.js:406
> +

Seems like there's one extra newline at the end of this file.