In WebKit1, we have this API for favicons: - static void QWebSettings::setIconDatabasePath(QString) - void QWebFrame::iconChanged() [signal] - QIcon QWebFrame::icon() const In WebKit2, the WebContext has a WebIconDatabase, so the most straightforward thing approach to expose the favicons through QWKContext. This patch adds the following API: - void QWKContext::setIconDatabasePath(QString) - void QWKContext::iconChangedForPageURL(QUrl) [signal] - QPixmap QWKContext::iconForPageURL(QUrl) This could be hooked into internally by QWKPage to provide e.g QWKPage::iconChanged() [signal] and QPixmap QWKPage::icon(), but I'd prefer to do that later as "convenience API".
Created attachment 90273 [details] Proposed patch Proposed patch. This does not cover the case where an icon is in already in the cache, though this could be added in a follow-up patch. QWebFrame::iconChanged() didn't work like that until very recently either.. :) @Brady: In Qt's WebKit1 API, we always emit the iconChanged() signal, also after an icon is retrieved from cache. Do you have any suggestions how to accomplish this in WebKit2?
Attachment 90273 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/UIProcess/API/qt/qwkcontext.cpp:25: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit2/UIProcess/API/qt/qwkcontext.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit2/UIProcess/API/qt/qwkcontext.cpp:32: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit2/UIProcess/API/qt/qwkcontext.cpp:57: One space before end of line comments [whitespace/comments] [5] Source/WebKit2/UIProcess/API/qt/qwkcontext.cpp:58: One space before end of line comments [whitespace/comments] [5] Source/WebKit2/UIProcess/API/qt/ClientImpl.h:56: The parameter name "iconDatabase" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit2/UIProcess/API/qt/ClientImpl.h:57: The parameter name "iconDatabase" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 7 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 90273 [details] Proposed patch 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.
Created attachment 93799 [details] Proposed patch v2 Same patch with QIcon instead of QPixmap as discussed. We're still missing a way to disable the icon database once enabled, so I've left a FIXME about that.
Attachment 93799 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/UIProcess/API/qt/qwkcontext.cpp:25: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit2/UIProcess/API/qt/qwkcontext.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit2/UIProcess/API/qt/qwkcontext.cpp:32: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit2/UIProcess/API/qt/qwkcontext.cpp:57: One space before end of line comments [whitespace/comments] [5] Source/WebKit2/UIProcess/API/qt/qwkcontext.cpp:58: One space before end of line comments [whitespace/comments] [5] Total errors found: 5 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 93799 [details] Proposed patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=93799&action=review > Source/WebKit2/UIProcess/API/qt/ClientImpl.cpp:208 > +void qt_wk_didRemoveAllIcons(WKIconDatabaseRef iconDatabase, const void* clientInfo) > +{ > +} notImplemented() ?
Comment on attachment 93799 [details] Proposed patch v2 Clearing flags on attachment: 93799 Committed r86694: <http://trac.webkit.org/changeset/86694>
All reviewed patches have been landed. Closing bug.