Changeset 183015 in webkit

Timestamp:

Apr 20, 2015 9:49:43 AM (9 years ago)

Author:

peavo@outlook.com

Message:

Favicons are not always loaded.
​https://bugs.webkit.org/show_bug.cgi?id=143880

Reviewed by Darin Adler.

Source/WebCore:

If the favicon link element(s) in the document does not have a mime type,
the favicon is loaded from the domain root (/favicon.ico). If no favicon
exists at this location, the favicon loading will fail. This can be solved
by not demanding that the link element has a mime type.

Test: fast/dom/icon-url-without-mimetype.html

• loader/icon/IconController.cpp:

(WebCore::iconFromLinkElements): Return the chosen icon URL instead of a vector of URLs.
(WebCore::IconController::url):
(WebCore::iconsFromLinkElements): Deleted.

LayoutTests:

Added new test for icon link elements without mime type.

• fast/dom/icon-url-without-mimetype-expected.txt: Added.
• fast/dom/icon-url-without-mimetype.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/icon-url-without-mimetype-expected.txt (added)
• LayoutTests/fast/dom/icon-url-without-mimetype.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/icon/IconController.cpp (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-04-20  Per Arne Vollan  <peavo@outlook.com>
+
+        Favicons are not always loaded.
+        https://bugs.webkit.org/show_bug.cgi?id=143880
+
+        Reviewed by Darin Adler.
+
+        Added new test for icon link elements without mime type.
+
+        * fast/dom/icon-url-without-mimetype-expected.txt: Added.
+        * fast/dom/icon-url-without-mimetype.html: Added.
+
 2015-04-20  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-04-20  Per Arne Vollan  <peavo@outlook.com>
+
+        Favicons are not always loaded.
+        https://bugs.webkit.org/show_bug.cgi?id=143880
+
+        Reviewed by Darin Adler.
+
+        If the favicon link element(s) in the document does not have a mime type,
+        the favicon is loaded from the domain root (/favicon.ico). If no favicon
+        exists at this location, the favicon loading will fail. This can be solved
+        by not demanding that the link element has a mime type.
+
+        Test: fast/dom/icon-url-without-mimetype.html
+
+        * loader/icon/IconController.cpp:
+        (WebCore::iconFromLinkElements): Return the chosen icon URL instead of a vector of URLs.
+        (WebCore::IconController::url):
+        (WebCore::iconsFromLinkElements): Deleted.
+
 2015-04-20  Martin Robinson  <mrobinson@igalia.com>
 

trunk/Source/WebCore/loader/icon/IconController.cpp

@@ -53 +53 @@
 namespace WebCore {
 
-enum class LinkElementSelector { All, WithMIMEType };
-
 IconController::IconController(Frame& frame)
     : m_frame(frame)
@@ -64 +62 @@
 }
 
-// FIXME: Given how this is used, there's no real need to use a vector here.
-// Should straighten this out and tighten up the code.
-static Vector<URL> iconsFromLinkElements(Frame& frame, LinkElementSelector selector)
-{
-    Vector<URL> result;
+static URL iconFromLinkElements(Frame& frame)
+{
+    // This function returns the first icon with a mime type.
+    // If no icon with mime type exists, the last icon is returned.
+    // It may make more sense to always return the last icon,
+    // but this implementation is consistent with previous behavior.
+
+    URL result;
 
     auto* document = frame.document();
@@ -83 +84 @@
         if (linkElement.href().isEmpty())
             continue;
-        if (selector == LinkElementSelector::WithMIMEType && linkElement.type().isEmpty())
-            continue;
-        result.append(linkElement.href());
-    }
-
-    // Put last icon seen at the front. This helps us implement a rule that says that icons seen later should take precedence.
-    result.reverse();
+        result = linkElement.href();
+        if (!linkElement.type().isEmpty())
+            break;
+    }
 
     return result;
@@ -99 +97 @@
         return URL();
 
-    // FIXME: This implements a rule that says "first icon seen with a MIME type wins".
-    // But that is not consistent with the comment in iconsFromLinkElements that says
-    // that icons seen *later* should take precedence.
-    URL icon;
-    for (auto& candidate : iconsFromLinkElements(m_frame, LinkElementSelector::WithMIMEType))
-        icon = candidate;
+    auto icon = iconFromLinkElements(m_frame);
     if (!icon.isEmpty())
         return icon;
@@ -116 +109 @@
         return icon;
     }
-
-    for (auto& candidate : iconsFromLinkElements(m_frame, LinkElementSelector::All))
-        return candidate;
 
     return URL();