[webkit-reviews] review denied: [Bug 58937] [Qt][WK2] Initial support for favicons. : [Attachment 90273] Proposed patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Andreas Kling
<kling at webkit.org>'s request for review:
Bug 58937: [Qt][WK2] Initial support for favicons.
https://bugs.webkit.org/show_bug.cgi?id=58937

Attachment 90273: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=90273&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=90273&action=review

Looks good to me, as discussed. r- because of the nitpicks :)

> Source/WebKit2/UIProcess/API/qt/qwkcontext.cpp:86
> +    // FIXME: Should we disable the icon database if path.isEmpty() (like
the WebKit1 API does?)

Yes :)

> Source/WebKit2/UIProcess/API/qt/qwkcontext.h:41
> +    QPixmap iconForPageURL(const QUrl&) const;

This should be a QIcon, as discussed at the Review-a-thon.