<link rel="apple-touch-icon"> tag is not honored on CNN.com, workflowy.com etc
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Mar 2013 00:42:32 +0000 (00:42 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Mar 2013 00:42:32 +0000 (00:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=109061

Patch by Ruslan Abdikeev <aruslan@chromium.org> on 2013-03-04
Reviewed by Adam Barth.

.:

* Source/autotools/symbols.filter:

Source/WebCore:

Test: fast/dom/icon-url-list-apple-touch.html

Added iconTypes parameter to Document::iconURLs().
Added Document::shortcutIconURLs() with original semantics of iconURLs().
Fixed IconController.cpp to provide iconTypesMask to iconURLs().
Renamed iconTypes to iconTypesMask to make the meaning clearer.

* WebCore.exp.in:
* dom/Document.cpp:
(WebCore::Document::shortcutIconURLs):
(WebCore):
(WebCore::Document::iconURLs):
* dom/Document.h:
(Document):
* loader/icon/IconController.cpp:
(WebCore::IconController::iconURL):
(WebCore::IconController::urlsForTypes):
* testing/Internals.cpp:
(WebCore::Internals::iconURLs):
(WebCore::Internals::shortcutIconURLs):
(WebCore):
(WebCore::Internals::allIconURLs):
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

Added test for apple-touch-icon in allIconURLs().
Changed iconURLs() to shortcutIconURLs().

* fast/dom/icon-url-change.html:
* fast/dom/icon-url-list-apple-touch-expected.txt: Added.
* fast/dom/icon-url-list-apple-touch.html: Added.
* fast/dom/icon-url-list.html:
* fast/dom/icon-url-property.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@144696 268f45cc-cd09-0410-ab3c-d52691b4dbfc

23 files changed:
ChangeLog
LayoutTests/ChangeLog
LayoutTests/fast/dom/icon-url-change.html
LayoutTests/fast/dom/icon-url-list-apple-touch-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/icon-url-list-apple-touch.html [new file with mode: 0644]
LayoutTests/fast/dom/icon-url-list.html
LayoutTests/fast/dom/icon-url-property.html
LayoutTests/platform/chromium-android/fast/dom/icon-url-list-apple-touch-expected.txt [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/loader/icon/IconController.cpp
Source/WebCore/loader/icon/IconController.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl
Source/WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in
Source/WebKit/chromium/public/WebFrame.h
Source/WebKit/chromium/src/WebFrameImpl.cpp
Source/WebKit/chromium/src/WebFrameImpl.h
Source/WebKit/win/WebKit.vcproj/WebKitExports.def.in
Source/autotools/symbols.filter

index 4aea99b..f99b8d1 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2013-03-04  Ruslan Abdikeev  <aruslan@chromium.org>
+
+        <link rel="apple-touch-icon"> tag is not honored on CNN.com, workflowy.com etc
+        https://bugs.webkit.org/show_bug.cgi?id=109061
+
+        Reviewed by Adam Barth.
+
+        * Source/autotools/symbols.filter:
+
 2013-03-04  Kunihiko Sakamoto  <ksakamoto@chromium.org>
 
         Add build flag for FontLoader
index 60781c5..33f1edb 100644 (file)
@@ -1,3 +1,19 @@
+2013-03-04  Ruslan Abdikeev  <aruslan@chromium.org>
+
+        <link rel="apple-touch-icon"> tag is not honored on CNN.com, workflowy.com etc
+        https://bugs.webkit.org/show_bug.cgi?id=109061
+
+        Reviewed by Adam Barth.
+
+        Added test for apple-touch-icon in allIconURLs().
+        Changed iconURLs() to shortcutIconURLs().
+
+        * fast/dom/icon-url-change.html:
+        * fast/dom/icon-url-list-apple-touch-expected.txt: Added.
+        * fast/dom/icon-url-list-apple-touch.html: Added.
+        * fast/dom/icon-url-list.html:
+        * fast/dom/icon-url-property.html:
+
 2013-02-27  Jeffrey Pfau  <jpfau@apple.com>
 
         Cache partitioning does not affect iframe MainResources
index 82d534e..993272e 100644 (file)
@@ -45,7 +45,7 @@ function runTests() {
 
     // check that the URL list in the document is as we expect
     var expectedURLs = "http://test.com/oldfavicon.ico";
-    var iconURLs = window.internals.iconURLs(document);
+    var iconURLs = window.internals.shortcutIconURLs(document);
     if (expectedURLs == iconURLs[0])
         testPassed('URL list matches expected');
     else
diff --git a/LayoutTests/fast/dom/icon-url-list-apple-touch-expected.txt b/LayoutTests/fast/dom/icon-url-list-apple-touch-expected.txt
new file mode 100644 (file)
index 0000000..77442f0
--- /dev/null
@@ -0,0 +1,3 @@
+Tests that all favicons and touch icons (if ENABLE(TOUCH_ICON_LOADING)) are in document.iconURLs()
+http://test.com/oldfavicon.ico
+
diff --git a/LayoutTests/fast/dom/icon-url-list-apple-touch.html b/LayoutTests/fast/dom/icon-url-list-apple-touch.html
new file mode 100644 (file)
index 0000000..941230d
--- /dev/null
@@ -0,0 +1,28 @@
+<html>
+<head>
+<title>Original Title</title>
+<link rel="shortcut icon" type="image/x-icon" href="http://test.com/oldfavicon.ico"/>
+<link rel="apple-touch-icon" type="image/png" href="http://test.com/i/touch.png"/>
+<link rel="apple-touch-icon-precomposed" type="image/png" href="http://test.com/i/touch-precomposed.png"/>
+<script src="../js/resources/js-test-pre.js"></script>
+<script>
+function runTests() {
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
+    debug('Tests that all favicons and touch icons (if ENABLE(TOUCH_ICON_LOADING)) are in document.iconURLs()');
+
+    // Fetch the actual list of icon URLs.
+    var actualURLs = window.internals.allIconURLs(document);
+
+    // Print out the URL list in the document to match it against the expected list.
+    // Note that the expected order is reverse to ensure that icons seen later
+    // take precedence as required by the spec.
+    for (var i = 0; i < actualURLs.length; i++)
+        debug(actualURLs[i]);
+}
+</script>
+</head>
+<body onload='runTests();'>
+</body>
+</html>
index dda00e7..6d3dfda 100644 (file)
@@ -48,7 +48,7 @@ function runTests() {
     var expectedURL0 = "http://test.com/barfavicon.ico";
     var expectedURL1 = "http://test.com/foofavicon.ico";
     var expectedURL2 = "http://test.com/newfavicon.ico";
-    var iconURLs = window.internals.iconURLs(document);
+    var iconURLs = window.internals.shortcutIconURLs(document);
     if (expectedURL0 == iconURLs[0] && expectedURL1 == iconURLs[1] && expectedURL2 == iconURLs[2]) 
         testPassed('URL list matches expected');
     else
index 8a6b9c2..2d1c012 100644 (file)
@@ -43,7 +43,7 @@ function runTests() {
 
     // check that the URL list in the document is as we expect
     var expectedURLs = "http://test.com/newfavicon.ico";
-    var iconURLs = window.internals.iconURLs(document);
+    var iconURLs = window.internals.shortcutIconURLs(document);
     if (expectedURLs == iconURLs[0])
         debugOutput('PASS - URL list matches expected');
     else
diff --git a/LayoutTests/platform/chromium-android/fast/dom/icon-url-list-apple-touch-expected.txt b/LayoutTests/platform/chromium-android/fast/dom/icon-url-list-apple-touch-expected.txt
new file mode 100644 (file)
index 0000000..24dbc4d
--- /dev/null
@@ -0,0 +1,5 @@
+Tests that all favicons and touch icons (if ENABLE(TOUCH_ICON_LOADING)) are in document.iconURLs()
+http://test.com/i/touch-precomposed.png
+http://test.com/i/touch.png
+http://test.com/oldfavicon.ico
+
index 975b949..18f295b 100644 (file)
@@ -1,3 +1,35 @@
+2013-03-04  Ruslan Abdikeev  <aruslan@chromium.org>
+
+        <link rel="apple-touch-icon"> tag is not honored on CNN.com, workflowy.com etc
+        https://bugs.webkit.org/show_bug.cgi?id=109061
+
+        Reviewed by Adam Barth.
+
+        Test: fast/dom/icon-url-list-apple-touch.html
+
+        Added iconTypes parameter to Document::iconURLs().
+        Added Document::shortcutIconURLs() with original semantics of iconURLs().
+        Fixed IconController.cpp to provide iconTypesMask to iconURLs().
+        Renamed iconTypes to iconTypesMask to make the meaning clearer.
+
+        * WebCore.exp.in:
+        * dom/Document.cpp:
+        (WebCore::Document::shortcutIconURLs):
+        (WebCore):
+        (WebCore::Document::iconURLs):
+        * dom/Document.h:
+        (Document):
+        * loader/icon/IconController.cpp:
+        (WebCore::IconController::iconURL):
+        (WebCore::IconController::urlsForTypes):
+        * testing/Internals.cpp:
+        (WebCore::Internals::iconURLs):
+        (WebCore::Internals::shortcutIconURLs):
+        (WebCore):
+        (WebCore::Internals::allIconURLs):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2013-02-27  Jeffrey Pfau  <jpfau@apple.com>
 
         Cache partitioning does not affect iframe MainResources
index b399172..33799ca 100644 (file)
@@ -982,7 +982,8 @@ __ZN7WebCore8Document36updateLayoutIgnorePendingStylesheetsEv
 __ZN7WebCore8Document4headEv
 __ZN7WebCore8FormData28removeGeneratedFilesIfNeededEv
 __ZN7WebCore8FormData6decodeERN3WTF7DecoderE
-__ZN7WebCore8Document8iconURLsEv
+__ZN7WebCore8Document8iconURLsEi
+__ZN7WebCore8Document16shortcutIconURLsEv
 __ZN7WebCore8FormData6createEPKvm
 __ZN7WebCore8FormDataD1Ev
 __ZN7WebCore8Gradient12addColorStopEfRKNS_5ColorE
index c36e65c..5b8c646 100644 (file)
@@ -4499,14 +4499,19 @@ PassRefPtr<XPathResult> Document::evaluate(const String& expression,
     return m_xpathEvaluator->evaluate(expression, contextNode, resolver, type, result, ec);
 }
 
-const Vector<IconURL>& Document::iconURLs()
+const Vector<IconURL>& Document::shortcutIconURLs()
+{
+    // Include any icons where type = link, rel = "shortcut icon".
+    return iconURLs(Favicon);
+}
+
+const Vector<IconURL>& Document::iconURLs(int iconTypesMask)
 {
     m_iconURLs.clear();
 
     if (!head() || !(head()->children()))
         return m_iconURLs;
 
-    // Include any icons where type = link, rel = "shortcut icon".
     RefPtr<HTMLCollection> children = head()->children();
     unsigned int length = children->length();
     for (unsigned int i = 0; i < length; ++i) {
@@ -4514,7 +4519,7 @@ const Vector<IconURL>& Document::iconURLs()
         if (!child->hasTagName(linkTag))
             continue;
         HTMLLinkElement* linkElement = static_cast<HTMLLinkElement*>(child);
-        if (linkElement->iconType() != Favicon)
+        if (!(linkElement->iconType() & iconTypesMask))
             continue;
         if (linkElement->href().isEmpty())
             continue;
index d8b9863..06363d3 100644 (file)
@@ -943,7 +943,8 @@ public:
     bool hasNodesWithPlaceholderStyle() const { return m_hasNodesWithPlaceholderStyle; }
     void setHasNodesWithPlaceholderStyle() { m_hasNodesWithPlaceholderStyle = true; }
 
-    const Vector<IconURL>& iconURLs();
+    const Vector<IconURL>& shortcutIconURLs();
+    const Vector<IconURL>& iconURLs(int iconTypesMask);
     void addIconURL(const String& url, const String& mimeType, const String& size, IconType);
 
     void setUseSecureKeyboardEntryWhenActive(bool);
index 436a97c..8f23f6d 100644 (file)
@@ -69,33 +69,31 @@ KURL IconController::url()
 IconURL IconController::iconURL(IconType iconType) const
 {
     IconURL result;
-    const Vector<IconURL>& iconURLs = m_frame->document()->iconURLs();
+    const Vector<IconURL>& iconURLs = m_frame->document()->iconURLs(iconType);
     Vector<IconURL>::const_iterator iter(iconURLs.begin());
     for (; iter != iconURLs.end(); ++iter) {
-        if (iter->m_iconType == iconType) {
-            if (result.m_iconURL.isEmpty() || !iter->m_mimeType.isEmpty())
-                result = *iter;
-        }
+        if (result.m_iconURL.isEmpty() || !iter->m_mimeType.isEmpty())
+            result = *iter;
     }
 
     return result;
 }
 
-IconURLs IconController::urlsForTypes(int iconTypes)
+IconURLs IconController::urlsForTypes(int iconTypesMask)
 {
     IconURLs iconURLs;
     if (m_frame->tree() && m_frame->tree()->parent())
         return iconURLs;
         
-    if (iconTypes & Favicon && !appendToIconURLs(Favicon, &iconURLs))
+    if (iconTypesMask & Favicon && !appendToIconURLs(Favicon, &iconURLs))
         iconURLs.append(defaultURL(Favicon));
 
 #if ENABLE(TOUCH_ICON_LOADING)
     int missedIcons = 0;
-    if (iconTypes & TouchPrecomposedIcon)
+    if (iconTypesMask & TouchPrecomposedIcon)
         missedIcons += appendToIconURLs(TouchPrecomposedIcon, &iconURLs) ? 0:1;
 
-    if (iconTypes & TouchIcon)
+    if (iconTypesMask & TouchIcon)
       missedIcons += appendToIconURLs(TouchIcon, &iconURLs) ? 0:1;
 
     // Only return the default touch icons when the both were required and neither was gotten.
@@ -106,11 +104,8 @@ IconURLs IconController::urlsForTypes(int iconTypes)
 #endif
 
     // Finally, append all remaining icons of this type.
-    const Vector<IconURL>& allIconURLs = m_frame->document()->iconURLs();
+    const Vector<IconURL>& allIconURLs = m_frame->document()->iconURLs(iconTypesMask);
     for (Vector<IconURL>::const_iterator iter = allIconURLs.begin(); iter != allIconURLs.end(); ++iter) {
-        if (!(iter->m_iconType & iconTypes))
-            continue;
-
         int i;
         int iconCount = iconURLs.size();
         for (i = 0; i < iconCount; ++i) {
index b2caba2..61dfa76 100644 (file)
@@ -48,7 +48,7 @@ public:
     ~IconController();
 
     KURL url();
-    IconURLs urlsForTypes(int iconTypes);
+    IconURLs urlsForTypes(int iconTypesMask);
     IconURL iconURL(IconType) const;
 
     void startLoader();
index 3ddeff7..d1a7e99 100644 (file)
@@ -1766,9 +1766,9 @@ int Internals::pageNumber(Element* element, float pageWidth, float pageHeight)
     return PrintContext::pageNumberForElement(element, FloatSize(pageWidth, pageHeight));
 }
 
-Vector<String> Internals::iconURLs(Document* document) const
+Vector<String> Internals::iconURLs(Document* document, int iconTypesMask) const
 {
-    Vector<IconURL> iconURLs = document->iconURLs();
+    Vector<IconURL> iconURLs = document->iconURLs(iconTypesMask);
     Vector<String> array;
 
     Vector<IconURL>::const_iterator iter(iconURLs.begin());
@@ -1778,6 +1778,16 @@ Vector<String> Internals::iconURLs(Document* document) const
     return array;
 }
 
+Vector<String> Internals::shortcutIconURLs(Document* document) const
+{
+    return iconURLs(document, Favicon);
+}
+
+Vector<String> Internals::allIconURLs(Document* document) const
+{
+    return iconURLs(document, Favicon | TouchIcon | TouchPrecomposedIcon);
+}
+
 int Internals::numberOfPages(float pageWidth, float pageHeight)
 {
     if (!frame())
index 4a26a95..eec383a 100644 (file)
@@ -251,7 +251,8 @@ public:
     String counterValue(Element*);
 
     int pageNumber(Element*, float pageWidth = 800, float pageHeight = 600);
-    Vector<String> iconURLs(Document*) const;
+    Vector<String> shortcutIconURLs(Document*) const;
+    Vector<String> allIconURLs(Document*) const;
 
     int numberOfPages(float pageWidthInPixels = 800, float pageHeightInPixels = 600);
     String pageProperty(String, int, ExceptionCode& = ASSERT_NO_EXCEPTION) const;
@@ -298,6 +299,7 @@ private:
     explicit Internals(Document*);
     Document* contextDocument() const;
     Frame* frame() const;
+    Vector<String> iconURLs(Document*, int iconTypesMask) const;
 
     DocumentMarker* markerAt(Node*, const String& markerType, unsigned index, ExceptionCode&);
 #if ENABLE(INSPECTOR)
index 58bd3c1..372b098 100644 (file)
 
     DOMString counterValue(in Element element);
     long pageNumber(in Element element, in [Optional] float pageWidth, in [Optional] float pageHeight);
-    DOMString[] iconURLs(in Document document);
+    DOMString[] shortcutIconURLs(in Document document);
+    DOMString[] allIconURLs(in Document document);
     long numberOfPages(in [Optional] double pageWidthInPixels, in [Optional] double pageHeightInPixels);
     DOMString pageProperty(in DOMString propertyName, in long pageNumber) raises (DOMException);
     DOMString pageSizeAndMarginsInPixels(in long pageIndex, in long width, in long height, in long marginTop, in long marginRight, in long marginBottom, in long marginLeft) raises (DOMException);
index e64b0fd..3d527bf 100644 (file)
@@ -326,7 +326,8 @@ EXPORTS
         ?setAllowsRoundingHacks@TextRun@WebCore@@SAX_N@Z
         ?registerURLSchemeAsBypassingContentSecurityPolicy@SchemeRegistry@WebCore@@SAXABVString@WTF@@@Z
         ?removeURLSchemeRegisteredAsBypassingContentSecurityPolicy@SchemeRegistry@WebCore@@SAXABVString@WTF@@@Z
-        ?iconURLs@Document@WebCore@@QAEABV?$Vector@UIconURL@WebCore@@$0A@@WTF@@XZ
+        ?shortcutIconURLs@Document@WebCore@@QAEABV?$Vector@UIconURL@WebCore@@$0A@@WTF@@XZ
+        ?iconURLs@Document@WebCore@@QAEABV?$Vector@UIconURL@WebCore@@$0A@@WTF@@H@Z
         ?numberOfPages@PrintContext@WebCore@@SAHPAVFrame@2@ABVFloatSize@2@@Z
         ?pageProperty@PrintContext@WebCore@@SA?AVString@WTF@@PAVFrame@2@PBDH@Z
         ?pageSizeAndMarginsInPixels@PrintContext@WebCore@@SA?AVString@WTF@@PAVFrame@2@HHHHHHH@Z
index 812ad6e..e74efeb 100644 (file)
@@ -130,10 +130,10 @@ public:
     virtual long long identifier() const = 0;
 
     // The urls of the given combination types of favicon (if any) specified by
-    // the document loaded in this frame. The iconTypes is a bit-mask of
+    // the document loaded in this frame. The iconTypesMask is a bit-mask of
     // WebIconURL::Type values, used to select from the available set of icon
     // URLs
-    virtual WebVector<WebIconURL> iconURLs(int iconTypes) const = 0;
+    virtual WebVector<WebIconURL> iconURLs(int iconTypesMask) const = 0;
 
 
     // Geometry -----------------------------------------------------------
index 3cdf503..da59eaa 100644 (file)
@@ -574,12 +574,12 @@ long long WebFrameImpl::identifier() const
     return m_identifier;
 }
 
-WebVector<WebIconURL> WebFrameImpl::iconURLs(int iconTypes) const
+WebVector<WebIconURL> WebFrameImpl::iconURLs(int iconTypesMask) const
 {
     // The URL to the icon may be in the header. As such, only
     // ask the loader for the icon if it's finished loading.
     if (frame()->loader()->state() == FrameStateComplete)
-        return frame()->loader()->icon()->urlsForTypes(iconTypes);
+        return frame()->loader()->icon()->urlsForTypes(iconTypesMask);
     return WebVector<WebIconURL>();
 }
 
index b09bbba..97c621d 100644 (file)
@@ -78,7 +78,7 @@ public:
     virtual WebString assignedName() const;
     virtual void setName(const WebString&);
     virtual long long identifier() const;
-    virtual WebVector<WebIconURL> iconURLs(int iconTypes) const;
+    virtual WebVector<WebIconURL> iconURLs(int iconTypesMask) const;
     virtual WebSize scrollOffset() const;
     virtual void setScrollOffset(const WebSize&);
     virtual WebSize minimumScrollOffset() const;
index 0604d37..b1aaf43 100644 (file)
@@ -328,7 +328,8 @@ EXPORTS
         ?setAllowsRoundingHacks@TextRun@WebCore@@SAX_N@Z
         ?registerURLSchemeAsBypassingContentSecurityPolicy@SchemeRegistry@WebCore@@SAXABVString@WTF@@@Z
         ?removeURLSchemeRegisteredAsBypassingContentSecurityPolicy@SchemeRegistry@WebCore@@SAXABVString@WTF@@@Z
-        ?iconURLs@Document@WebCore@@QAEABV?$Vector@UIconURL@WebCore@@$0A@@WTF@@XZ
+        ?shortcutIconURLs@Document@WebCore@@QAEABV?$Vector@UIconURL@WebCore@@$0A@@WTF@@XZ
+        ?iconURLs@Document@WebCore@@QAEABV?$Vector@UIconURL@WebCore@@$0A@@WTF@@H@Z
         ?numberOfPages@PrintContext@WebCore@@SAHPAVFrame@2@ABVFloatSize@2@@Z
         ?pageProperty@PrintContext@WebCore@@SA?AVString@WTF@@PAVFrame@2@PBDH@Z
         ?pageSizeAndMarginsInPixels@PrintContext@WebCore@@SA?AVString@WTF@@PAVFrame@2@HHHHHHH@Z
index dbeaf47..7ba25d0 100644 (file)
@@ -42,7 +42,8 @@ _ZN7WebCore4toJSEPN3JSC9ExecStateEPNS_17JSDOMGlobalObjectEPNS_5RangeE;
 _ZN7WebCore5Range6createEN3WTF10PassRefPtrINS_8DocumentEEENS2_INS_4NodeEEEiS6_i;
 _ZN7WebCore5RangeD1Ev;
 _ZN7WebCore8Document36updateLayoutIgnorePendingStylesheetsEv;
-_ZN7WebCore8Document8iconURLsEv;
+_ZN7WebCore8Document8iconURLsEi;
+_ZN7WebCore8Document16shortcutIconURLsEv;
 _ZN7WebCore8Settings19minDOMTimerIntervalEv;
 _ZN7WebCore8Settings22setMinDOMTimerIntervalEd;
 _ZN7WebCore9HTMLNames8inputTagE;