.: Expose an export for the iconUrl list so Internals can use it
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jul 2012 01:37:36 +0000 (01:37 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jul 2012 01:37:36 +0000 (01:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=88665

Patch by Pete Williamson <petewil@google.com> on 2012-07-09
Reviewed by Kent Tamura.

* Source/autotools/symbols.filter: export iconURLs

Source/WebCore: Changed the behavior of iconURLs to always recalculate the list.
https://bugs.webkit.org/show_bug.cgi?id=88665

Patch by Pete Williamson <petewil@google.com> on 2012-07-09
Reviewed by Kent Tamura..

As it turns out, it can contain stale URLs in the case that some script
manipulates the DOM, which breaks scripts trying to reset the favicon
URL. Also added a method in Internals to allow tests to get the list of
icon

Tests: fast/dom/icon-url-change.html
       fast/dom/icon-url-list.html

* WebCore.exp.in: export Document::iconURLs on the mac for the Internals class
* dom/Document.cpp:
(WebCore::Document::iconURLs): Changed the method to recalculate the iconURL list every time
(WebCore::Document::addIconURL): we no longer need to add to the internal list since we recalculate it
(WebCore::Document::setUseSecureKeyboardEntryWhenActive): removed extra whitespace
* dom/Document.h:
(Document): removed the addIconURL method which is no longer used
* html/HTMLLinkElement.cpp:
(WebCore::HTMLLinkElement::iconType): exposed the icon type with an accessor
(WebCore):
(WebCore::HTMLLinkElement::iconSizes): exposed the icon sizes with an accessor
* html/HTMLLinkElement.h:
(HTMLLinkElement): declared the icon type and size accessors
* testing/Internals.cpp:
(WebCore::Internals::iconURLs): made a method to be used by unit tests for inspecting the icon URL list
(WebCore):
* testing/Internals.h:
(Internals): declared the method for unit testing the icon URL list
* testing/Internals.idl: exported the Document::iconURLs function

Source/WebKit2: Export the iconURL list to make it available to the Internals class for testing
https://bugs.webkit.org/show_bug.cgi?id=88665

Patch by Pete Williamson <petewil@google.com> on 2012-07-09
Reviewed by Kent Tamura.

* win/WebKit2.def: export the DocumentL::iconURLs function

LayoutTests: Add some new unit tests to test the favicon changing dynamically
https://bugs.webkit.org/show_bug.cgi?id=88665

Patch by Pete Williamson <petewil@google.com> on 2012-07-09
Reviewed by Kent Tamura.

* fast/dom/icon-url-change-expected.txt: Added.
* fast/dom/icon-url-change.html: Added a new test for changing the favicon dynamically
* fast/dom/icon-url-list-expected.txt: Added.
* fast/dom/icon-url-list.html: Added a new test for multiple favicons in the HTML header
* fast/dom/icon-url-property-expected.txt: update unit test expectations
* fast/dom/icon-url-property.html: update and enable existing favicon test
* platform/chromium/TestExpectations: reenable the url-property test

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

21 files changed:
ChangeLog
LayoutTests/ChangeLog
LayoutTests/fast/dom/icon-url-change-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/icon-url-change.html [new file with mode: 0644]
LayoutTests/fast/dom/icon-url-list-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/icon-url-list.html [new file with mode: 0644]
LayoutTests/fast/dom/icon-url-property-expected.txt
LayoutTests/fast/dom/icon-url-property.html
LayoutTests/platform/chromium/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/html/HTMLLinkElement.cpp
Source/WebCore/html/HTMLLinkElement.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl
Source/WebKit2/ChangeLog
Source/WebKit2/win/WebKit2.def
Source/autotools/symbols.filter

index dafa8e5..729fca6 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2012-07-09  Pete Williamson  <petewil@google.com>
+
+        Expose an export for the iconUrl list so Internals can use it
+        https://bugs.webkit.org/show_bug.cgi?id=88665
+
+        Reviewed by Kent Tamura.
+
+        * Source/autotools/symbols.filter: export iconURLs
+
 2012-07-09  Mike Lattanzio  <mlattanzio@rim.com>
 
         [BlackBerry] meta viewport initial-scale doesn't factor in device pixel ratio
index 8dd586f..35c030d 100644 (file)
@@ -1,3 +1,18 @@
+2012-07-09  Pete Williamson  <petewil@google.com>
+
+        Add some new unit tests to test the favicon changing dynamically
+        https://bugs.webkit.org/show_bug.cgi?id=88665
+
+        Reviewed by Kent Tamura.
+
+        * fast/dom/icon-url-change-expected.txt: Added.
+        * fast/dom/icon-url-change.html: Added a new test for changing the favicon dynamically
+        * fast/dom/icon-url-list-expected.txt: Added.
+        * fast/dom/icon-url-list.html: Added a new test for multiple favicons in the HTML header
+        * fast/dom/icon-url-property-expected.txt: update unit test expectations
+        * fast/dom/icon-url-property.html: update and enable existing favicon test
+        * platform/chromium/TestExpectations: reenable the url-property test
+
 2012-07-09  Alice Cheng  <alice_cheng@apple.com>
 
         Editing: Autocorrection in blockquotes causes text to break out of quote
diff --git a/LayoutTests/fast/dom/icon-url-change-expected.txt b/LayoutTests/fast/dom/icon-url-change-expected.txt
new file mode 100644 (file)
index 0000000..43f2b66
--- /dev/null
@@ -0,0 +1,7 @@
+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
+Setting icon URL back to: http://test.com/oldfavicon.ico
+Original iconURL is still: http://test.com/oldfavicon.ico
+PASS URL list matches expected
+
diff --git a/LayoutTests/fast/dom/icon-url-change.html b/LayoutTests/fast/dom/icon-url-change.html
new file mode 100644 (file)
index 0000000..82d534e
--- /dev/null
@@ -0,0 +1,60 @@
+<html>
+<head>
+<title>Original Title</title>
+<link rel="shortcut icon" type="image/x-icon" href="http://test.com/oldfavicon.ico"/>
+<script src="../js/resources/js-test-pre.js"></script>
+<script>
+
+function setFavIcon(iconURL) {
+    var docHead = document.getElementsByTagName("head")[0];
+    var links = docHead.getElementsByTagName("link");
+    for (var i = 0; i < links.length; ++i) {
+        var link = links[i];
+        if (link.type=="image/x-icon" && link.rel=="shortcut icon") {
+            docHead.removeChild(link);
+            break; // Assuming only one match at most.
+        }
+    }
+    var link = document.createElement("link");
+    link.type = "image/x-icon";
+    link.rel = "shortcut icon";
+    link.href = iconURL;
+    docHead.appendChild(link);
+}
+
+function runTests() {
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
+    iconURL = document.getElementsByTagName("head")[0].getElementsByTagName("link")[0].href;
+    debug('Original iconURL is: ' + iconURL);
+
+    // change icon to new icon
+    newURL = 'http://test.com/newfavicon.ico';
+    debug('Setting new icon URL to: ' + newURL);
+    setFavIcon(newURL);
+    iconURL = document.getElementsByTagName("link")[0].href;
+    debug('New iconURL is: ' + iconURL);
+
+    // change icon back to old icon and ensure it changes properly
+    oldURL = 'http://test.com/oldfavicon.ico';
+    debug('Setting icon URL back to: ' + oldURL);
+    setFavIcon(oldURL);
+    iconURL = document.getElementsByTagName("link")[0].href;
+    debug('Original iconURL is still: ' + iconURL);
+
+    // 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);
+    if (expectedURLs == iconURLs[0])
+        testPassed('URL list matches expected');
+    else
+        testFailed('URL list does not match expected');
+}
+
+</script>
+</head>
+<body onload='runTests();'>
+</div>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/icon-url-list-expected.txt b/LayoutTests/fast/dom/icon-url-list-expected.txt
new file mode 100644 (file)
index 0000000..739adcd
--- /dev/null
@@ -0,0 +1,5 @@
+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
+PASS URL list matches expected
+
diff --git a/LayoutTests/fast/dom/icon-url-list.html b/LayoutTests/fast/dom/icon-url-list.html
new file mode 100644 (file)
index 0000000..dda00e7
--- /dev/null
@@ -0,0 +1,63 @@
+<html>
+<head>
+<title>Original Title</title>
+<link rel="shortcut icon" type="image/x-icon" href="http://test.com/oldfavicon.ico"/>
+<link rel="shortcut icon" type="image/x-icon" href="http://test.com/foofavicon.ico"/>
+<link rel="shortcut icon" type="image/x-icon" href="http://test.com/barfavicon.ico"/>
+<script src="../js/resources/js-test-pre.js"></script>
+<script>
+
+function setFavIcon(iconURL) {
+    var docHead = document.getElementsByTagName("head")[0];
+
+    // set up a new node for the new iconURL
+    var newLink = document.createElement("link");
+    newLink.type = "image/x-icon";
+    newLink.rel = "shortcut icon";
+    newLink.href = iconURL;
+
+    var links = docHead.getElementsByTagName("link");
+    for (var i = 0; i < links.length; ++i) {
+        var oldLink = links[i];
+        if (oldLink.type=="image/x-icon" && oldLink.rel=="shortcut icon") {
+          // if we find the child, replace it with the new node.
+          docHead.replaceChild(newLink, oldLink);
+          return; // Assuming only one match at most.
+        }
+    }
+
+    // if we didn't find the icon URL link, add it now.
+    docHead.appendChild(newLink);
+}
+
+function runTests() {
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
+    iconURL = document.getElementsByTagName("link")[0].href;
+    debug('Original iconURL is: ' + iconURL);
+
+    // change icon to new icon
+    newURL = 'http://test.com/newfavicon.ico';
+    debug('Setting new icon URL to: ' + newURL);
+    setFavIcon(newURL);
+    iconURL = document.getElementsByTagName("link")[0].href
+    debug('New iconURL is: ' + iconURL);
+
+    // check that the URL list in the document is as we expect
+    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);
+    if (expectedURL0 == iconURLs[0] && expectedURL1 == iconURLs[1] && expectedURL2 == iconURLs[2]) 
+        testPassed('URL list matches expected');
+    else
+        testFailed('URL list does not match expected');
+}
+
+</script>
+</head>
+<body onload='runTests();'>
+</div>
+</body>
+</html>
index 13ba05f..0c0ffce 100644 (file)
@@ -1,4 +1,4 @@
-main frame - didChangeIcons
 Original iconURL is: http://test.com/oldfavicon.ico
-Setting new icon URL to: http://test.com/newfavion.ico
-New iconURL is: http://test.com/newfavion.ico
+Setting new icon URL to: http://test.com/newfavicon.ico
+New iconURL is: http://test.com/newfavicon.ico
+PASS - URL list matches expected
index eb51d7d..8a6b9c2 100644 (file)
@@ -29,18 +29,26 @@ function setFavIcon(iconURL) {
 }
 
 function runTests() {
-    if (window.testRunner) {
+    if (window.testRunner)
         testRunner.dumpAsText();
-        testRunner.dumpIconChanges();
-    }
+
     iconURL = document.getElementsByTagName("head")[0].getElementsByTagName("link")[0].href;
     debugOutput ('Original iconURL is: ' + iconURL);
-    newURL = 'http://test.com/newfavion.ico';
+    newURL = 'http://test.com/newfavicon.ico';
     debugOutput ('Setting new icon URL to: ' + newURL);
     setFavIcon(newURL);
     iconURL = document.getElementsByTagName("head")[0].getElementsByTagName("link")[0].href;
 
     debugOutput ('New iconURL is: ' + iconURL);
+
+    // 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);
+    if (expectedURLs == iconURLs[0])
+        debugOutput('PASS - URL list matches expected');
+    else
+        debugOutput('FAIL - URL list does not match expected');
+
 }
 
 </script>
index aab96c7..baae146 100644 (file)
@@ -1873,8 +1873,6 @@ BUGWK71758 : fast/url/port.html = TEXT PASS
 // // Started failing at r58152
 // BUGWK38038 : fast/url/file-http-base.html = TEXT
 
-BUGWK33812 SKIP : fast/dom/icon-url-property.html = PASS
-
 // New layoutTestController function added at r57993
 BUGCR42696 : http/tests/xmlhttprequest/cross-origin-authorization-with-embedder.html = TIMEOUT
 
index f86b098..1699f08 100644 (file)
@@ -1,3 +1,38 @@
+2012-07-09  Pete Williamson  <petewil@google.com>
+
+        Changed the behavior of iconURLs to always recalculate the list.
+        https://bugs.webkit.org/show_bug.cgi?id=88665
+
+        Reviewed by Kent Tamura..
+
+        As it turns out, it can contain stale URLs in the case that some script
+        manipulates the DOM, which breaks scripts trying to reset the favicon
+        URL. Also added a method in Internals to allow tests to get the list of
+        icon
+
+        Tests: fast/dom/icon-url-change.html
+               fast/dom/icon-url-list.html
+
+        * WebCore.exp.in: export Document::iconURLs on the mac for the Internals class
+        * dom/Document.cpp:
+        (WebCore::Document::iconURLs): Changed the method to recalculate the iconURL list every time
+        (WebCore::Document::addIconURL): we no longer need to add to the internal list since we recalculate it
+        (WebCore::Document::setUseSecureKeyboardEntryWhenActive): removed extra whitespace
+        * dom/Document.h:
+        (Document): removed the addIconURL method which is no longer used
+        * html/HTMLLinkElement.cpp:
+        (WebCore::HTMLLinkElement::iconType): exposed the icon type with an accessor
+        (WebCore):
+        (WebCore::HTMLLinkElement::iconSizes): exposed the icon sizes with an accessor
+        * html/HTMLLinkElement.h:
+        (HTMLLinkElement): declared the icon type and size accessors
+        * testing/Internals.cpp:
+        (WebCore::Internals::iconURLs): made a method to be used by unit tests for inspecting the icon URL list
+        (WebCore):
+        * testing/Internals.h:
+        (Internals): declared the method for unit testing the icon URL list
+        * testing/Internals.idl: exported the Document::iconURLs function
+
 2012-07-09  Ryosuke Niwa  <rniwa@webkit.org>
 
         Gcc build fix after r122174.
index 30f3f2d..55bb75c 100644 (file)
@@ -831,6 +831,7 @@ __ZN7WebCore8Document26pageSizeAndMarginsInPixelsEiRNS_7IntSizeERiS3_S3_S3_
 __ZN7WebCore8Document27removeMediaCanStartListenerEPNS_21MediaCanStartListenerE
 __ZN7WebCore8Document36updateLayoutIgnorePendingStylesheetsEv
 __ZN7WebCore8Document4headEv
+__ZN7WebCore8Document8iconURLsEv
 __ZN7WebCore8FormData6createEPKvm
 __ZN7WebCore8FormDataD1Ev
 __ZN7WebCore8Gradient12addColorStopEfRKNS_5ColorE
index bc5eabb..0a66d72 100644 (file)
@@ -4865,8 +4865,30 @@ PassRefPtr<XPathResult> Document::evaluate(const String& expression,
     return m_xpathEvaluator->evaluate(expression, contextNode, resolver, type, result, ec);
 }
 
-const Vector<IconURL>& Document::iconURLs() const
+const Vector<IconURL>& Document::iconURLs()
 {
+    static const char* const iconMIMEType = "image/x-icon";
+
+    m_iconURLs.clear();
+
+    // 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) {
+        Node* child = children->item(i);
+        if (!child->hasTagName(linkTag))
+            continue;
+        HTMLLinkElement* linkElement = static_cast<HTMLLinkElement*>(child);
+        if (!equalIgnoringCase(linkElement->type(), iconMIMEType) || linkElement->iconType() != Favicon)
+            continue;
+        if (linkElement->href().isEmpty())
+            continue;
+
+        // Put it at the front to ensure that icons seen later take precedence as required by the spec.
+        IconURL newURL(linkElement->href(), linkElement->iconSizes(), linkElement->type(), linkElement->iconType());
+        m_iconURLs.prepend(newURL);
+    }
+
     return m_iconURLs;
 }
 
@@ -4877,7 +4899,6 @@ void Document::addIconURL(const String& url, const String& mimeType, const Strin
 
     // FIXME - <rdar://problem/4727645> - At some point in the future, we might actually honor the "mimeType"
     IconURL newURL(KURL(ParsedURLString, url), sizes, mimeType, iconType);
-    m_iconURLs.append(newURL);
 
     if (Frame* f = frame()) {
         IconURL iconURL = f->loader()->icon()->iconURL(iconType);
@@ -4890,7 +4911,7 @@ void Document::setUseSecureKeyboardEntryWhenActive(bool usesSecureKeyboard)
 {
     if (m_useSecureKeyboardEntryWhenActive == usesSecureKeyboard)
         return;
-        
+
     m_useSecureKeyboardEntryWhenActive = usesSecureKeyboard;
     m_frame->selection()->updateSecureKeyboardEntryIfActive();
 }
index 87dfa32..679f345 100644 (file)
@@ -939,7 +939,7 @@ public:
     
     void setHasNodesWithPlaceholderStyle() { m_hasNodesWithPlaceholderStyle = true; }
 
-    const Vector<IconURL>& iconURLs() const;
+    const Vector<IconURL>& iconURLs();
     void addIconURL(const String& url, const String& mimeType, const String& size, IconType);
 
     void setUseSecureKeyboardEntryWhenActive(bool);
index f378de5..096ecca 100644 (file)
@@ -418,6 +418,16 @@ String HTMLLinkElement::type() const
     return getAttribute(typeAttr);
 }
 
+IconType HTMLLinkElement::iconType() const
+{
+    return m_relAttribute.m_iconType;
+}
+
+String HTMLLinkElement::iconSizes() const
+{
+    return m_sizes->toString();
+}
+
 void HTMLLinkElement::addSubresourceAttributeURLs(ListHashSet<KURL>& urls) const
 {
     HTMLElement::addSubresourceAttributeURLs(urls);
index bb69e12..e330e73 100644 (file)
@@ -55,6 +55,11 @@ public:
 
     String type() const;
 
+    IconType iconType() const;
+
+    // the icon size string as parsed from the HTML attribute
+    String iconSizes() const;
+
     CSSStyleSheet* sheet() const { return m_sheet.get(); }
 
     bool styleSheetIsLoading() const;
index 2270b4a..af2650c 100644 (file)
@@ -1138,6 +1138,18 @@ String Internals::counterValue(Element* element)
     return counterValueForElement(element);
 }
 
+PassRefPtr<DOMStringList> Internals::iconURLs(Document* document) const
+{
+    Vector<IconURL> iconURLs = document->iconURLs();
+    RefPtr<DOMStringList> stringList = DOMStringList::create();
+
+    Vector<IconURL>::const_iterator iter(iconURLs.begin());
+    for (; iter != iconURLs.end(); ++iter)
+        stringList->append(iter->m_iconURL.string());
+
+    return stringList.release();
+}
+
 #if ENABLE(FULLSCREEN_API)
 void Internals::webkitWillEnterFullScreenForElement(Document* document, Element* element)
 {
index beac6f0..aed11b5 100644 (file)
@@ -184,6 +184,7 @@ public:
 #endif
 
     String counterValue(Element*);
+    PassRefPtr<DOMStringList> iconURLs(Document*) const;
 
 #if ENABLE(FULLSCREEN_API)
     void webkitWillEnterFullScreenForElement(Document*, Element*);
index 2f17f55..2d430e2 100644 (file)
@@ -158,6 +158,7 @@ module window {
         [Conditional=INSPECTOR] sequence<String> consoleMessageArgumentCounts(in Document document);
 
         DOMString counterValue(in Element element);
+        DOMString[] iconURLs(in Document document);
 
 #if defined(ENABLE_FULLSCREEN_API) && ENABLE_FULLSCREEN_API
         void webkitWillEnterFullScreenForElement(in Document document, in Element element);
index d04bed3..0cb5dee 100644 (file)
@@ -1,3 +1,12 @@
+2012-07-09  Pete Williamson  <petewil@google.com>
+
+        Export the iconURL list to make it available to the Internals class for testing
+        https://bugs.webkit.org/show_bug.cgi?id=88665
+
+        Reviewed by Kent Tamura.
+
+        * win/WebKit2.def: export the DocumentL::iconURLs function
+
 2012-07-09  Christophe Dumez  <christophe.dumez@intel.com>
 
         [WK2] Add missing Battery Status API integration to WebContext and WebPage
index 6cfafc5..958db76 100644 (file)
@@ -271,3 +271,4 @@ EXPORTS
         ?jsStringSlowCase@WebCore@@YA?AVJSValue@JSC@@PAVExecState@3@AAV?$HashMap@PAVStringImpl@WTF@@V?$Weak@VJSString@JSC@@@JSC@@U?$PtrHash@PAVStringImpl@WTF@@@2@U?$HashTraits@PAVStringImpl@WTF@@@2@U?$HashTraits@V?$Weak@VJSString@JSC@@@JSC@@@2@@WTF@@PAVStringImpl@6@@Z        
         ?registerURLSchemeAsBypassingContentSecurityPolicy@SchemeRegistry@WebCore@@SAXABVString@WTF@@@Z
         ?removeURLSchemeRegisteredAsBypassingContentSecurityPolicy@SchemeRegistry@WebCore@@SAXABVString@WTF@@@Z
+        ?iconURLs@Document@WebCore@@QAEABV?$Vector@UIconURL@WebCore@@$0A@@WTF@@XZ
\ No newline at end of file
index f0ca5a3..d50cc04 100644 (file)
@@ -37,6 +37,7 @@ _ZN7WebCore4toJSEPN3JSC9ExecStateEPNS_17JSDOMGlobalObjectEPNS_5RangeE;
 _ZN7WebCore5Range6createEN3WTF10PassRefPtrINS_8DocumentEEENS2_INS_4NodeEEEiS6_i;
 _ZN7WebCore5RangeD1Ev;
 _ZN7WebCore8Document36updateLayoutIgnorePendingStylesheetsEv;
+_ZN7WebCore8Document8iconURLsEv;
 _ZN7WebCore9HTMLNames8inputTagE;
 _ZN7WebCore9HTMLNames11textareaTagE;
 _ZN7WebCore10JSDocument10putVirtualEPN3JSC9ExecStateERKNS1_10IdentifierENS1_7JSValueERNS1_15PutPropertySlotE;