summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPeter Varga <pvarga@inf.u-szeged.hu>2019-02-11 16:38:47 +0100
committerPeter Varga <pvarga@inf.u-szeged.hu>2019-02-25 07:14:32 +0000
commit837347e1901086a9757b5aca7e5bbee7b0d8e20e (patch)
treee0bf0ed4ae2523f87e96e65b57858c5e5f3405d7
parent598bfc652edffb047948e01c3c76eb9e0cb7db4e (diff)
Fix handling of touch icons when it is disabled
Store the icon type in a bitfield because the same icon URL might be used for various types on same page. This way webengine won't ignore to download a default icon what is also set as a touch icon when touch icons are disabled. Moreover, do not store the icon types from the previous page because a subsequent page might use the same icon URL but with different type. With this change the type of the cached icons are updated after each page load. Fixes: QTBUG-70081 Change-Id: I8031a740b07b0c6a8e5759a994f386b13ce87be2 Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
-rw-r--r--src/core/favicon_manager.cpp20
-rw-r--r--src/core/favicon_manager.h15
-rw-r--r--src/core/type_conversion.cpp2
-rw-r--r--tests/auto/quick/qmltests/data/tst_favicon.qml33
-rw-r--r--tests/auto/widgets/faviconmanager/tst_faviconmanager.cpp38
5 files changed, 94 insertions, 14 deletions
diff --git a/src/core/favicon_manager.cpp b/src/core/favicon_manager.cpp
index de6a0f18..f7ba858c 100644
--- a/src/core/favicon_manager.cpp
+++ b/src/core/favicon_manager.cpp
@@ -253,7 +253,7 @@ void FaviconManager::update(const QList<FaviconInfo> &candidates)
const QList<FaviconInfo> &faviconInfoList = getFaviconInfoList(true /* candidates only */);
for (auto it = faviconInfoList.cbegin(), end = faviconInfoList.cend(); it != end; ++it) {
- if (!touchIconsEnabled && it->type != FaviconInfo::Favicon)
+ if (!touchIconsEnabled && !(it->type & FaviconInfo::Favicon))
continue;
if (it->isValid())
@@ -272,6 +272,14 @@ void FaviconManager::update(const QList<FaviconInfo> &candidates)
void FaviconManager::updateCandidates(const QList<FaviconInfo> &candidates)
{
+ // Invalidate types of the already stored candidate icons because it might differ
+ // among pages.
+ for (FaviconInfo candidateFaviconInfo : candidates) {
+ const QUrl &candidateUrl = candidateFaviconInfo.url;
+ if (m_faviconInfoMap.contains(candidateUrl))
+ m_faviconInfoMap[candidateUrl].type = FaviconInfo::InvalidIcon;
+ }
+
m_candidateCount = candidates.count();
for (FaviconInfo candidateFaviconInfo : candidates) {
const QUrl &candidateUrl = candidateFaviconInfo.url;
@@ -279,8 +287,8 @@ void FaviconManager::updateCandidates(const QList<FaviconInfo> &candidates)
if (!m_faviconInfoMap.contains(candidateUrl))
m_faviconInfoMap.insert(candidateUrl, candidateFaviconInfo);
else {
- // The same icon can be used for more than one page with different types.
- m_faviconInfoMap[candidateUrl].type = candidateFaviconInfo.type;
+ // The same icon URL can be used for different types.
+ m_faviconInfoMap[candidateUrl].type |= candidateFaviconInfo.type;
}
m_faviconInfoMap[candidateUrl].candidate = true;
@@ -311,7 +319,7 @@ QUrl FaviconManager::candidateIconUrl(bool touchIconsEnabled) const
unsigned bestArea = 0;
for (auto it = faviconInfoList.cbegin(), end = faviconInfoList.cend(); it != end; ++it) {
- if (!touchIconsEnabled && it->type != FaviconInfo::Favicon)
+ if (!touchIconsEnabled && !(it->type & FaviconInfo::Favicon))
continue;
if (it->isValid() && bestArea < area(it->size)) {
@@ -331,7 +339,7 @@ void FaviconManager::generateCandidateIcon(bool touchIconsEnabled)
const QList<FaviconInfo> &faviconInfoList = getFaviconInfoList(true /* candidates only */);
for (auto it = faviconInfoList.cbegin(), end = faviconInfoList.cend(); it != end; ++it) {
- if (!touchIconsEnabled && it->type != FaviconInfo::Favicon)
+ if (!touchIconsEnabled && !(it->type & FaviconInfo::Favicon))
continue;
if (!it->isValid() || !it->isDownloaded())
@@ -373,7 +381,7 @@ FaviconInfo::FaviconInfo(const FaviconInfo &other)
{
}
-FaviconInfo::FaviconInfo(const QUrl &url, FaviconInfo::FaviconType type)
+FaviconInfo::FaviconInfo(const QUrl &url, FaviconInfo::FaviconTypeFlags type)
: url(url)
, type(type)
, size(QSize(0, 0))
diff --git a/src/core/favicon_manager.h b/src/core/favicon_manager.h
index f9758d0f..60d194c4 100644
--- a/src/core/favicon_manager.h
+++ b/src/core/favicon_manager.h
@@ -84,23 +84,24 @@ class WebContentsAdapterClient;
// Based on src/3rdparty/chromium/content/public/common/favicon_url.h
class QWEBENGINECORE_PRIVATE_EXPORT FaviconInfo {
public:
- enum FaviconType {
- InvalidIcon,
- Favicon,
- TouchIcon,
- TouchPrecomposedIcon
+ enum FaviconTypeFlag {
+ InvalidIcon = 0,
+ Favicon = 1 << 0,
+ TouchIcon = 1 << 1,
+ TouchPrecomposedIcon = 1 << 2
};
+ Q_DECLARE_FLAGS(FaviconTypeFlags, FaviconTypeFlag);
FaviconInfo();
FaviconInfo(const FaviconInfo &);
- FaviconInfo(const QUrl &, FaviconInfo::FaviconType);
+ FaviconInfo(const QUrl &, FaviconInfo::FaviconTypeFlags);
~FaviconInfo();
bool isValid() const;
bool isDownloaded() const;
QUrl url;
- FaviconType type;
+ FaviconTypeFlags type;
// Stores the largest size in case of multi-size icon
QSize size;
bool candidate;
diff --git a/src/core/type_conversion.cpp b/src/core/type_conversion.cpp
index d35426ac..f69b2a29 100644
--- a/src/core/type_conversion.cpp
+++ b/src/core/type_conversion.cpp
@@ -216,7 +216,7 @@ int flagsFromModifiers(Qt::KeyboardModifiers modifiers)
return modifierFlags;
}
-FaviconInfo::FaviconType toQt(content::FaviconURL::IconType type)
+FaviconInfo::FaviconTypeFlags toQt(content::FaviconURL::IconType type)
{
switch (type) {
case content::FaviconURL::IconType::kFavicon:
diff --git a/tests/auto/quick/qmltests/data/tst_favicon.qml b/tests/auto/quick/qmltests/data/tst_favicon.qml
index 563a87c8..50a41238 100644
--- a/tests/auto/quick/qmltests/data/tst_favicon.qml
+++ b/tests/auto/quick/qmltests/data/tst_favicon.qml
@@ -375,5 +375,38 @@ TestWebEngineView {
faviconImage.destroy()
}
+
+ function test_touchIconWithSameURL()
+ {
+ WebEngine.settings.touchIconsEnabled = false;
+
+ var icon = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=";
+
+ webEngineView.loadHtml(
+ "<html>" +
+ "<link rel='icon' type='image/png' href='" + icon + "'/>" +
+ "<link rel='apple-touch-icon' type='image/png' href='" + icon + "'/>" +
+ "</html>"
+ );
+ verify(webEngineView.waitForLoadSucceeded());
+
+ // The default favicon has to be loaded even if its URL is also set as a touch icon while touch icons are disabled.
+ tryCompare(iconChangedSpy, "count", 1);
+ compare(webEngineView.icon.toString().replace(/^image:\/\/favicon\//, ''), icon);
+
+ iconChangedSpy.clear();
+
+ webEngineView.loadHtml(
+ "<html>" +
+ "<link rel='apple-touch-icon' type='image/png' href='" + icon + "'/>" +
+ "</html>"
+ );
+ verify(webEngineView.waitForLoadSucceeded());
+
+ // This page only has a touch icon. With disabled touch icons we don't expect any icon to be shown even if the same icon
+ // was loaded previously.
+ tryCompare(iconChangedSpy, "count", 1);
+ verify(!webEngineView.icon.toString().replace(/^image:\/\/favicon\//, ''));
+ }
}
}
diff --git a/tests/auto/widgets/faviconmanager/tst_faviconmanager.cpp b/tests/auto/widgets/faviconmanager/tst_faviconmanager.cpp
index 606d05d9..540c8d50 100644
--- a/tests/auto/widgets/faviconmanager/tst_faviconmanager.cpp
+++ b/tests/auto/widgets/faviconmanager/tst_faviconmanager.cpp
@@ -62,6 +62,7 @@ private Q_SLOTS:
void downloadTouchIconsEnabled_data();
void downloadTouchIconsEnabled();
void dynamicFavicon();
+ void touchIconWithSameURL();
private:
QWebEngineView *m_view;
@@ -508,6 +509,43 @@ void tst_FaviconManager::dynamicFavicon()
}
}
+void tst_FaviconManager::touchIconWithSameURL()
+{
+ m_page->settings()->setAttribute(QWebEngineSettings::TouchIconsEnabled, false);
+
+ const QString icon("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=");
+ QSignalSpy loadFinishedSpy(m_page, SIGNAL(loadFinished(bool)));
+ QSignalSpy iconUrlChangedSpy(m_page, SIGNAL(iconUrlChanged(QUrl)));
+ QSignalSpy iconChangedSpy(m_page, SIGNAL(iconChanged(QIcon)));
+
+ m_page->setHtml("<html>"
+ "<link rel='icon' type='image/png' href='" + icon + "'/>"
+ "<link rel='apple-touch-icon' type='image/png' href='" + icon + "'/>"
+ "</html>");
+ QTRY_COMPARE(loadFinishedSpy.count(), 1);
+
+ // The default favicon has to be loaded even if its URL is also set as a touch icon while touch icons are disabled.
+ QTRY_COMPARE(iconUrlChangedSpy.count(), 1);
+ QCOMPARE(m_page->iconUrl().toString(), icon);
+ QTRY_COMPARE(iconChangedSpy.count(), 1);
+
+ loadFinishedSpy.clear();
+ iconUrlChangedSpy.clear();
+ iconChangedSpy.clear();
+
+ m_page->setHtml("<html>"
+ "<link rel='apple-touch-icon' type='image/png' href='" + icon + "'/>"
+ "</html>");
+ QTRY_COMPARE(loadFinishedSpy.count(), 1);
+
+ // This page only has a touch icon. With disabled touch icons we don't expect any icon to be shown even if the same icon
+ // was loaded previously.
+ QTRY_COMPARE(iconUrlChangedSpy.count(), 1);
+ QVERIFY(m_page->iconUrl().toString().isEmpty());
+ QTRY_COMPARE(iconChangedSpy.count(), 1);
+
+}
+
QTEST_MAIN(tst_FaviconManager)
#include "tst_faviconmanager.moc"