[webkit-reviews] review denied: [Bug 64580] Add support for download='filename' in anchors : [Attachment 100911] first cut for review

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied sadrul at chromium.org's request for
review:
Bug 64580: Add support for download='filename' in anchors
https://bugs.webkit.org/show_bug.cgi?id=64580

Attachment 100911: first cut for review
https://bugs.webkit.org/attachment.cgi?id=100911&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=100911&action=review


Some notes below.  You'll also need to add this to HTMLAnchorElement.idl so
that it can be feature-detected.

> Source/WebCore/html/HTMLAnchorElement.cpp:500
> +    String url = stripLeadingAndTrailingHTMLSpaces(getAttribute(hrefAttr));

getAttribute -> fastGetAttibute, I believe.

> Source/WebCore/html/HTMLAnchorElement.cpp:503
> +    bool download = hasAttribute("download");

"download" should be added to the appropriate HTMLNames.in file, alongside the
other attributes.

> Source/WebCore/html/HTMLAnchorElement.cpp:504
> +    String name = download ? getAttribute("download") : "";

We're using "" as distinct from String() here?	Also probably should be
fastGetAttribute.

> Source/WebCore/loader/FrameLoader.cpp:264
> +void FrameLoader::urlSelected(const KURL& url, const String& passedTarget,
PassRefPtr<Event> triggeringEvent, bool lockHistory, bool lockBackForwardList,
ReferrerPolicy referrerPolicy, bool shouldDownload, const String* downloadName)


const String& rather than const String*

> Source/WebCore/platform/network/ResourceRequestBase.h:159
> +	   bool shouldDownload() const { return m_shouldDownload; }
> +	   void setShouldDownload(bool should) { m_shouldDownload = should; }
> +
> +	   String downloadName() const { return m_downloadName; }
> +	   void setDownloadName(String name) { m_downloadName = name; }

You can't add things to ResourceRequestBase that don't round-trip through
NSURLRequest (or whatever that type is called).