Title: [146284] trunk
- Revision
- 146284
- Author
- le...@chromium.org
- Date
- 2013-03-19 18:09:38 -0700 (Tue, 19 Mar 2013)
Log Message
FrameLoader::didChangeIcons isn't called when the favicon is changed.
https://bugs.webkit.org/show_bug.cgi?id=112080
Reviewed by Dmitry Titov.
Source/WebCore:
Test: fast/dom/icon-url-property.html
* dom/Document.cpp:
(WebCore::Document::addIconURL): Made the callback for didChangeIcons
happen whenever there are favicon changes instead of filtering it
and have to be in sync about what hosts care about. As far as I could
tell no hosts rely on this callback except for Chromium, so in general
doing less work here may potentially help a small amount for other
ports.
LayoutTests:
* fast/dom/icon-url-property-expected.txt:
* fast/dom/icon-url-property.html: Add some more calls
to trigger didChangeIcons callbacks.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (146283 => 146284)
--- trunk/LayoutTests/ChangeLog 2013-03-20 00:57:28 UTC (rev 146283)
+++ trunk/LayoutTests/ChangeLog 2013-03-20 01:09:38 UTC (rev 146284)
@@ -1,3 +1,14 @@
+2013-03-19 David Levin <le...@chromium.org>
+
+ FrameLoader::didChangeIcons isn't called when the favicon is changed.
+ https://bugs.webkit.org/show_bug.cgi?id=112080
+
+ Reviewed by Dmitry Titov.
+
+ * fast/dom/icon-url-property-expected.txt:
+ * fast/dom/icon-url-property.html: Add some more calls
+ to trigger didChangeIcons callbacks.
+
2013-03-19 Michael Pruett <mich...@68k.org>
Don't throw on infinity or NaN index in HTMLOptionsCollection.add()
Modified: trunk/LayoutTests/fast/dom/icon-url-property-expected.txt (146283 => 146284)
--- trunk/LayoutTests/fast/dom/icon-url-property-expected.txt 2013-03-20 00:57:28 UTC (rev 146283)
+++ trunk/LayoutTests/fast/dom/icon-url-property-expected.txt 2013-03-20 01:09:38 UTC (rev 146284)
@@ -1,4 +1,6 @@
main frame - didChangeIcons
+main frame - didChangeIcons
+main frame - didChangeIcons
Original iconURL is: http://test.com/oldfavicon.ico
Setting new icon URL to: http://test.com/newfavicon.ico
New iconURL is: http://test.com/newfavicon.ico
Modified: trunk/LayoutTests/fast/dom/icon-url-property.html (146283 => 146284)
--- trunk/LayoutTests/fast/dom/icon-url-property.html 2013-03-20 00:57:28 UTC (rev 146283)
+++ trunk/LayoutTests/fast/dom/icon-url-property.html 2013-03-20 01:09:38 UTC (rev 146284)
@@ -11,8 +11,20 @@
debugDiv.appendChild(div);
}
+function createFavIconElement(iconURL) {
+ var link = document.createElement("link");
+ link.type = "image/x-icon";
+ link.rel = "shortcut icon";
+ link.href = ""
+ return link;
+}
+
+function getHeadElement() {
+ return document.getElementsByTagName("head")[0];
+}
+
function setFavIcon(iconURL) {
- var docHead = document.getElementsByTagName("head")[0];
+ var docHead = getHeadElement();
var links = docHead.getElementsByTagName("link");
for (var i = 0; i < links.length; ++i) {
var link = links[i];
@@ -21,11 +33,7 @@
break; // Assuming only one match at most.
}
}
- var link = document.createElement("link");
- link.type = "image/x-icon";
- link.rel = "shortcut icon";
- link.href = ""
- docHead.appendChild(link);
+ docHead.appendChild(createFavIconElement(iconURL));
}
function runTests() {
@@ -52,6 +60,10 @@
else
debugOutput('FAIL - URL list does not match expected');
+ // Add some more fav icons to verify that didChangeIcons gets called.
+ var docHead = getHeadElement();
+ docHead.insertBefore(createFavIconElement("http://example.org/icon1.ico"), docHead.firstChild);
+ docHead.appendChild(createFavIconElement("http://example.org/icon2.ico"));
}
</script>
Modified: trunk/Source/WebCore/ChangeLog (146283 => 146284)
--- trunk/Source/WebCore/ChangeLog 2013-03-20 00:57:28 UTC (rev 146283)
+++ trunk/Source/WebCore/ChangeLog 2013-03-20 01:09:38 UTC (rev 146284)
@@ -1,3 +1,20 @@
+2013-03-19 David Levin <le...@chromium.org>
+
+ FrameLoader::didChangeIcons isn't called when the favicon is changed.
+ https://bugs.webkit.org/show_bug.cgi?id=112080
+
+ Reviewed by Dmitry Titov.
+
+ Test: fast/dom/icon-url-property.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::addIconURL): Made the callback for didChangeIcons
+ happen whenever there are favicon changes instead of filtering it
+ and have to be in sync about what hosts care about. As far as I could
+ tell no hosts rely on this callback except for Chromium, so in general
+ doing less work here may potentially help a small amount for other
+ ports.
+
2013-03-19 Michael Pruett <mich...@68k.org>
Don't throw on infinity or NaN index in HTMLOptionsCollection.add()
Modified: trunk/Source/WebCore/dom/Document.cpp (146283 => 146284)
--- trunk/Source/WebCore/dom/Document.cpp 2013-03-20 00:57:28 UTC (rev 146283)
+++ trunk/Source/WebCore/dom/Document.cpp 2013-03-20 01:09:38 UTC (rev 146284)
@@ -4583,19 +4583,16 @@
return m_iconURLs;
}
-void Document::addIconURL(const String& url, const String& mimeType, const String& sizes, IconType iconType)
+void Document::addIconURL(const String& url, const String&, const String&, IconType iconType)
{
if (url.isEmpty())
return;
- // FIXME - <rdar://problem/4727645> - At some point in the future, we might actually honor the "mimeType"
- IconURL newURL(KURL(ParsedURLString, url), sizes, mimeType, iconType);
+ Frame* f = frame();
+ if (!f)
+ return;
- if (Frame* f = frame()) {
- IconURL iconURL = f->loader()->icon()->iconURL(iconType);
- if (iconURL == newURL)
- f->loader()->didChangeIcons(iconType);
- }
+ f->loader()->didChangeIcons(iconType);
}
void Document::setUseSecureKeyboardEntryWhenActive(bool usesSecureKeyboard)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
