Changeset 122806 in webkit

Timestamp:

Jul 16, 2012 9:37:29 PM (12 years ago)

Author:

commit-queue@webkit.org

Message:

.: Expose an export for the iconUrl list so Internals can use it
​https://bugs.webkit.org/show_bug.cgi?id=88665

Patch by Pete Williamson <​petewil@google.com> on 2012-07-16
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-16
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-16
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-16
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

Location:

trunk

Files:

• ChangeLog (modified) (1 diff)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/icon-url-change-expected.txt (added)
• LayoutTests/fast/dom/icon-url-change.html (added)
• LayoutTests/fast/dom/icon-url-list-expected.txt (added)
• LayoutTests/fast/dom/icon-url-list.html (added)
• LayoutTests/fast/dom/icon-url-property-expected.txt (modified) (1 diff)
• LayoutTests/fast/dom/icon-url-property.html (modified) (2 diffs)
• LayoutTests/platform/chromium/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/WebCore.exp.in (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (3 diffs)
• Source/WebCore/dom/Document.h (modified) (1 diff)
• Source/WebCore/html/HTMLLinkElement.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLLinkElement.h (modified) (1 diff)
• Source/WebCore/testing/Internals.cpp (modified) (1 diff)
• Source/WebCore/testing/Internals.h (modified) (1 diff)
• Source/WebCore/testing/Internals.idl (modified) (1 diff)
• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/win/WebKit2.def (modified) (1 diff)
• Source/autotools/symbols.filter (modified) (1 diff)

trunk/ChangeLog

@@ +1 @@
+2012-07-16  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-16  Hajime Morrita  <morrita@chromium.org>
 

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-07-16  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-16  Kiran Muppala  <cmuppala@apple.com>
 

trunk/LayoutTests/fast/dom/icon-url-property-expected.txt

@@ -1 @@
-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

trunk/LayoutTests/fast/dom/icon-url-property.html

@@ -30 +30 @@
 
 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);
@@ -42 +41 @@
 
     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');
+
 }
 

trunk/LayoutTests/platform/chromium/TestExpectations

@@ -1875 +1875 @@
 // 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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-07-16  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-16  Hajime Morrita  <morrita@chromium.org>
 

trunk/Source/WebCore/WebCore.exp.in

@@ -835 +835 @@
 __ZN7WebCore8Document36updateLayoutIgnorePendingStylesheetsEv
 __ZN7WebCore8Document4headEv
+__ZN7WebCore8Document8iconURLsEv
 __ZN7WebCore8FormData6createEPKvm
 __ZN7WebCore8FormDataD1Ev

trunk/Source/WebCore/dom/Document.cpp

@@ -4895 +4895 @@
 }
 
-const Vector<IconURL>& Document::iconURLs() const
-{
+const Vector<IconURL>& Document::iconURLs()
+{
+    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) {
+        Node* child = children->item(i);
+        if (!child->hasTagName(linkTag))
+            continue;
+        HTMLLinkElement* linkElement = static_cast<HTMLLinkElement*>(child);
+        if (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;
 }
@@ -4907 +4930 @@
     // 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()) {
@@ -4920 +4942 @@
     if (m_useSecureKeyboardEntryWhenActive == usesSecureKeyboard)
         return;
-        
+
     m_useSecureKeyboardEntryWhenActive = usesSecureKeyboard;
     m_frame->selection()->updateSecureKeyboardEntryIfActive();

trunk/Source/WebCore/dom/Document.h

@@ -953 +953 @@
     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);
 

trunk/Source/WebCore/html/HTMLLinkElement.cpp

@@ -419 +419 @@
 }
 
+IconType HTMLLinkElement::iconType() const
+{
+    return m_relAttribute.m_iconType;
+}
+
+String HTMLLinkElement::iconSizes() const
+{
+    return m_sizes->toString();
+}
+
 void HTMLLinkElement::addSubresourceAttributeURLs(ListHashSet<KURL>& urls) const
 {

trunk/Source/WebCore/html/HTMLLinkElement.h

@@ -55 +55 @@
 
     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(); }

trunk/Source/WebCore/testing/Internals.cpp

@@ -1077 +1077 @@
 }
 
+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)

trunk/Source/WebCore/testing/Internals.h

@@ -186 +186 @@
 
     String counterValue(Element*);
+    PassRefPtr<DOMStringList> iconURLs(Document*) const;
 
 #if ENABLE(FULLSCREEN_API)

trunk/Source/WebCore/testing/Internals.idl

@@ -166 +166 @@
 
         DOMString counterValue(in Element element);
+        DOMString[] iconURLs(in Document document);
 
 #if defined(ENABLE_FULLSCREEN_API) && ENABLE_FULLSCREEN_API

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2012-07-16  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-16  Hajime Morrita  <morrita@chromium.org>
 

trunk/Source/WebKit2/win/WebKit2.def

@@ -274 +274 @@
         ?registerURLSchemeAsBypassingContentSecurityPolicy@SchemeRegistry@WebCore@@SAXABVString@WTF@@@Z
         ?removeURLSchemeRegisteredAsBypassingContentSecurityPolicy@SchemeRegistry@WebCore@@SAXABVString@WTF@@@Z
+        ?iconURLs@Document@WebCore@@QAEABV?$Vector@UIconURL@WebCore@@$0A@@WTF@@XZ

trunk/Source/autotools/symbols.filter

@@ -38 +38 @@
 _ZN7WebCore5RangeD1Ev;
 _ZN7WebCore8Document36updateLayoutIgnorePendingStylesheetsEv;
+_ZN7WebCore8Document8iconURLsEv;
 _ZN7WebCore9HTMLNames8inputTagE;
 _ZN7WebCore9HTMLNames11textareaTagE;