link elements should be able to fire more than one load / error event

https://bugs.webkit.org/show_bug.cgi?id=232309 Reviewed by Darin Adler. Based on a patch written by Chris Dumez. This patch makes link element emit more than one load event each time resource is loaded, and fixes the bug that setting rel content attribute to the same value resulted in the resource to be reloaded. New behavior matches that of Chrome and Firefox. * LayoutTests/TestExpectations: Unskip now passing tests. * LayoutTests/fast/dom/HTMLLinkElement/link-preload-load-once-expected.txt: Added. * LayoutTests/fast/dom/HTMLLinkElement/link-preload-load-once.html: Added. * LayoutTests/fast/dom/HTMLLinkElement/link-stylesheet-load-once-expected.txt: Added. * LayoutTests/fast/dom/HTMLLinkElement/link-stylesheet-load-once.html: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/document-metadata/the-link-element/link-multiple-error-events-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/document-metadata/the-link-element/link-multiple-load-events-expected.txt: * LayoutTests/platform/mac-wk1/fast/dom/HTMLLinkElement/link-preload-load-once-expected.txt: * LayoutTests/platform/win/fast/dom/HTMLLinkElement/link-preload-load-once-expected.txt: * Source/WebCore/html/HTMLLinkElement.cpp: (WebCore::HTMLLinkElement::HTMLLinkElement): (WebCore::HTMLLinkElement::parseAttribute): (WebCore::HTMLLinkElement::process): (WebCore::HTMLLinkElement::didFinishInsertingNode): We need to resolve URL again once this element is connected. (WebCore::HTMLLinkElement::notifyLoadedSheetAndAllCriticalSubresources): * Source/WebCore/html/HTMLLinkElement.h: * Source/WebCore/html/LinkRelAttribute.h: (WebCore::operator==): Added. (WebCore::operator!=): Added. Canonical link: https://commits.webkit.org/254290@main
WebKit · Sep 9, 2022 · 3775c2e · 3775c2e
1 parent eaf0a29
commit 3775c2e
Show file tree

Hide file tree

Showing 12 changed files with 153 additions and 21 deletions.
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -678,8 +678,6 @@ imported/w3c/web-platform-tests/html/browsers/sandboxing/sandbox-disallow-script
 imported/w3c/web-platform-tests/html/canvas/element/fill-and-stroke-styles/2d.pattern.transform.infinity.html [ Skip ]
 imported/w3c/web-platform-tests/html/rendering/replaced-elements/embedded-content-rendering-rules/audio-controls-002.html [ Skip ]
 imported/w3c/web-platform-tests/html/rendering/replaced-elements/svg-inline-sizing/svg-inline.html [ Skip ]
-imported/w3c/web-platform-tests/html/semantics/document-metadata/the-link-element/link-multiple-error-events.html [ Skip ]
-imported/w3c/web-platform-tests/html/semantics/document-metadata/the-link-element/link-multiple-load-events.html [ Skip ]
 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_navigate_ancestor-1.sub.html [ Skip ]
 imported/w3c/web-platform-tests/html/semantics/forms/historical-search-event.html [ Skip ]
 imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-summary-element/anchor-with-inline-element.html [ Skip ]

diff --git a/LayoutTests/fast/dom/HTMLLinkElement/link-preload-load-once-expected.txt b/LayoutTests/fast/dom/HTMLLinkElement/link-preload-load-once-expected.txt
@@ -0,0 +1,8 @@
+data:text/css,div { background: green; width: 100px; height: 100px } - willSendRequest <NSURLRequest URL data:text/css,div { background: green; width: 100px; height: 100px }, main document URL link-preload-load-once.html, http method GET> redirectResponse (null)
+link-preload-load-once.html - didFinishLoading
+data:text/css,div { background: green; width: 100px; height: 100px } - didReceiveResponse <NSURLResponse data:text/css,div { background: green; width: 100px; height: 100px }, http status code 200>
+data:text/css,div { background: green; width: 100px; height: 100px } - didFinishLoading
+This tests overriding rel content attribute with the same value.
+You should see 1 below:
+
+1
diff --git a/LayoutTests/fast/dom/HTMLLinkElement/link-preload-load-once.html b/LayoutTests/fast/dom/HTMLLinkElement/link-preload-load-once.html
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+    testRunner.dumpResourceLoadCallbacks();
+}
+
+let loadCount = 0;
+function didLoad(element)
+{
+    if (!loadCount) {
+        document.querySelector('link').rel = 'preload';
+        setTimeout(() => {
+            document.querySelector('link').rel = 'PreLoad';          
+        }, 100);
+        setTimeout(() => {
+            document.querySelector('link').rel = 'PreLoad ';          
+        }, 200);
+        setTimeout(() => {
+            if (window.testRunner)
+              testRunner.notifyDone();
+        }, 300);
+    }
+    ++loadCount;
+    result.innerHTML = loadCount;
+}
+
+</script>
+<link rel="preload" as="style" href="data:text/css,div { background: green; width: 100px; height: 100px }" onload="didLoad(this)">
+</head>
+<body>
+<p>This tests overriding rel content attribute with the same value.<br>
+You should see 1 below:</p>
+<div id="result"></div>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/HTMLLinkElement/link-stylesheet-load-once-expected.txt b/LayoutTests/fast/dom/HTMLLinkElement/link-stylesheet-load-once-expected.txt
@@ -0,0 +1,8 @@
+resources/link-stylesheet-load-once.css - willSendRequest <NSURLRequest URL resources/link-stylesheet-load-once.css, main document URL link-stylesheet-load-once.html, http method GET> redirectResponse (null)
+link-stylesheet-load-once.html - didFinishLoading
+resources/link-stylesheet-load-once.css - didReceiveResponse <NSURLResponse resources/link-stylesheet-load-once.css, http status code 0>
+resources/link-stylesheet-load-once.css - didFinishLoading
+This tests overriding rel content attribute with the same value.
+You should see 1 in a green box below:
+
+1
diff --git a/LayoutTests/fast/dom/HTMLLinkElement/link-stylesheet-load-once.html b/LayoutTests/fast/dom/HTMLLinkElement/link-stylesheet-load-once.html
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+    testRunner.dumpResourceLoadCallbacks();
+}
+
+let loadCount = 0;
+function didLoad(element)
+{
+    if (!loadCount) {
+        document.querySelector('link').rel = 'stylesheet';
+        setTimeout(() => document.querySelector('link').rel = 'StyleSheet', 50);
+        setTimeout(() => document.querySelector('link').rel = '  StyleSheet', 100);
+        setTimeout(() => document.querySelector('link').media = 'all', 150);
+        setTimeout(() => document.querySelector('link').media = 'All', 200);
+        setTimeout(() => document.querySelector('link').type = 'text/css', 250);
+        setTimeout(() => document.querySelector('link').href = 'resources/link-stylesheet-load-once.css', 300);
+        setTimeout(() => document.querySelector('link').href = '  ../HTMLLinkElement/resources/link-stylesheet-load-once.css', 350);
+        setTimeout(() => {
+            if (window.testRunner)
+              testRunner.notifyDone();
+        }, 400);
+    }
+    ++loadCount;
+    result.innerHTML = loadCount;
+}
+
+</script>
+<link rel="stylesheet" type="text/css" media="all" href="resources/link-stylesheet-load-once.css" onload="didLoad(this)">
+</head>
+<body>
+<p>This tests overriding rel content attribute with the same value.<br>
+You should see 1 in a green box below:</p>
+<div id="result"></div>
+</body>
+</html>
diff --git a/...html/semantics/document-metadata/the-link-element/link-multiple-error-events-expected.txt b/...html/semantics/document-metadata/the-link-element/link-multiple-error-events-expected.txt
@@ -1,5 +1,3 @@
 
-Harness Error (TIMEOUT), message = null
-
-TIMEOUT Check if the <link>'s error event fires for each stylesheet it fails to load Test timed out
+PASS Check if the <link>'s error event fires for each stylesheet it fails to load
 
diff --git a/.../html/semantics/document-metadata/the-link-element/link-multiple-load-events-expected.txt b/.../html/semantics/document-metadata/the-link-element/link-multiple-load-events-expected.txt
@@ -1,5 +1,3 @@
 
-Harness Error (TIMEOUT), message = null
-
-TIMEOUT Check if the <link>'s load event fires for each stylesheet it loads Test timed out
+PASS Check if the <link>'s load event fires for each stylesheet it loads
 
diff --git a/LayoutTests/platform/mac-wk1/fast/dom/HTMLLinkElement/link-preload-load-once-expected.txt b/LayoutTests/platform/mac-wk1/fast/dom/HTMLLinkElement/link-preload-load-once-expected.txt
@@ -0,0 +1,8 @@
+data:text/css,div%20%7B%20background:%20green;%20width:%20100px;%20height:%20100px%20%7D - willSendRequest <NSURLRequest URL data:text/css,div%20%7B%20background:%20green;%20width:%20100px;%20height:%20100px%20%7D, main document URL link-preload-load-once.html, http method GET> redirectResponse (null)
+link-preload-load-once.html - didFinishLoading
+data:text/css,div%20%7B%20background:%20green;%20width:%20100px;%20height:%20100px%20%7D - didReceiveResponse <NSURLResponse data:text/css,div%20%7B%20background:%20green;%20width:%20100px;%20height:%20100px%20%7D, http status code 0>
+data:text/css,div%20%7B%20background:%20green;%20width:%20100px;%20height:%20100px%20%7D - didFinishLoading
+This tests overriding rel content attribute with the same value.
+You should see 1 below:
+
+1
diff --git a/LayoutTests/platform/win/fast/dom/HTMLLinkElement/link-preload-load-once-expected.txt b/LayoutTests/platform/win/fast/dom/HTMLLinkElement/link-preload-load-once-expected.txt
@@ -0,0 +1,8 @@
+data:text/css,div { background: green; width: 100px; height: 100px } - willSendRequest <NSURLRequest URL data:text/css,div { background: green; width: 100px; height: 100px }, main document URL link-preload-load-once.html, http method GET> redirectResponse (null)
+link-preload-load-once.html - didFinishLoading
+data:text/css,div { background: green; width: 100px; height: 100px } - didReceiveResponse <NSURLResponse data:text/css,div { background: green; width: 100px; height: 100px }, http status code 0>
+data:text/css,div { background: green; width: 100px; height: 100px } - didFinishLoading
+This tests overriding rel content attribute with the same value.
+You should see 1 below:
+
+1
diff --git a/Source/WebCore/html/HTMLLinkElement.cpp b/Source/WebCore/html/HTMLLinkElement.cpp
@@ -92,7 +92,6 @@ inline HTMLLinkElement::HTMLLinkElement(const QualifiedName& tagName, Document&
     , m_disabledState(Unset)
     , m_loading(false)
     , m_createdByParser(createdByParser)
-    , m_firedLoad(false)
     , m_loadedResource(false)
     , m_isHandlingBeforeLoad(false)
     , m_allowPrefetchLoadAndErrorForTesting(false)
@@ -167,17 +166,26 @@ void HTMLLinkElement::setDisabledState(bool disabled)
 void HTMLLinkElement::parseAttribute(const QualifiedName& name, const AtomString& value)
 {
     if (name == relAttr) {
-        m_relAttribute = LinkRelAttribute(document(), value);
+        auto parsedRel = LinkRelAttribute(document(), value);
+        auto didMutateRel = parsedRel != m_relAttribute;
+        m_relAttribute = WTFMove(parsedRel);
         if (m_relList)
             m_relList->associatedAttributeValueChanged(value);
-        process();
+        if (didMutateRel)
+            process();
         return;
     }
     if (name == hrefAttr) {
+        URL url = getNonEmptyURLAttribute(hrefAttr);
+        if (url == m_url)
+            return;
+        m_url = WTFMove(url);
         process();
         return;
     }
     if (name == typeAttr) {
+        if (value == m_type)
+            return;
         m_type = value;
         process();
         return;
@@ -189,7 +197,10 @@ void HTMLLinkElement::parseAttribute(const QualifiedName& name, const AtomString
         return;
     }
     if (name == mediaAttr) {
-        m_media = value.string().convertToASCIILowercase();
+        auto media = value.string().convertToASCIILowercase();
+        if (media == m_media)
+            return;
+        m_media = WTFMove(media);
         process();
         if (m_sheet && !isDisabled())
             m_styleScope->didChangeActiveStyleSheetCandidates();
@@ -252,11 +263,9 @@ void HTMLLinkElement::process()
     if (m_isHandlingBeforeLoad)
         return;
 
-    URL url = getNonEmptyURLAttribute(hrefAttr);
-
     LinkLoadParameters params {
         m_relAttribute,
-        url,
+        m_url,
         attributeWithoutSynchronization(asAttr),
         attributeWithoutSynchronization(mediaAttr),
         attributeWithoutSynchronization(typeAttr),
@@ -281,7 +290,7 @@ void HTMLLinkElement::process()
 
     LOG_WITH_STREAM(StyleSheets, stream << "HTMLLinkElement " << this << " process() - treatAsStyleSheet " << treatAsStyleSheet);
 
-    if (m_disabledState != Disabled && treatAsStyleSheet && document().frame() && url.isValid()) {
+    if (m_disabledState != Disabled && treatAsStyleSheet && document().frame() && m_url.isValid()) {
         String charset = attributeWithoutSynchronization(charsetAttr);
         if (!PAL::TextEncoding { charset }.isValid())
             charset = document().charset();
@@ -322,7 +331,7 @@ void HTMLLinkElement::process()
         options.integrity = m_integrityMetadataForPendingSheetRequest;
         options.referrerPolicy = params.referrerPolicy;
 
-        auto request = createPotentialAccessControlRequest(WTFMove(url), WTFMove(options), document(), crossOrigin());
+        auto request = createPotentialAccessControlRequest(m_url, WTFMove(options), document(), crossOrigin());
         request.setPriority(WTFMove(priority));
         request.setCharset(WTFMove(charset));
         request.setInitiator(*this);
@@ -380,6 +389,7 @@ Node::InsertedIntoAncestorResult HTMLLinkElement::insertedIntoAncestor(Insertion
 
 void HTMLLinkElement::didFinishInsertingNode()
 {
+    m_url = getNonEmptyURLAttribute(hrefAttr);
     process();
 }
 
@@ -559,11 +569,8 @@ DOMTokenList& HTMLLinkElement::relList()
 
 void HTMLLinkElement::notifyLoadedSheetAndAllCriticalSubresources(bool errorOccurred)
 {
-    if (m_firedLoad)
-        return;
     m_loadedResource = !errorOccurred;
     linkLoadEventSender().dispatchEventSoon(*this);
-    m_firedLoad = true;
 }
 
 void HTMLLinkElement::startLoadingDynamicSheet()

diff --git a/Source/WebCore/html/HTMLLinkElement.h b/Source/WebCore/html/HTMLLinkElement.h
@@ -146,13 +146,13 @@ class HTMLLinkElement final : public HTMLElement, public CachedStyleSheetClient,
     String m_type;
     String m_media;
     String m_integrityMetadataForPendingSheetRequest;
+    URL m_url;
     std::unique_ptr<DOMTokenList> m_sizes;
     std::unique_ptr<DOMTokenList> m_relList;
     DisabledState m_disabledState;
     LinkRelAttribute m_relAttribute;
     bool m_loading : 1;
     bool m_createdByParser : 1;
-    bool m_firedLoad : 1;
     bool m_loadedResource : 1;
     bool m_isHandlingBeforeLoad : 1;
     bool m_allowPrefetchLoadAndErrorForTesting : 1;

diff --git a/Source/WebCore/html/LinkRelAttribute.h b/Source/WebCore/html/LinkRelAttribute.h
@@ -58,4 +58,24 @@ struct LinkRelAttribute {
     static bool isSupported(Document&, StringView);
 };
 
+inline bool operator==(const LinkRelAttribute& left, const LinkRelAttribute& right)
+{
+    return left.iconType == right.iconType
+        && left.isStyleSheet == right.isStyleSheet
+        && left.isAlternate == right.isAlternate
+        && left.isDNSPrefetch == right.isDNSPrefetch
+        && left.isLinkPreload == right.isLinkPreload
+        && left.isLinkPreconnect == right.isLinkPreconnect
+        && left.isLinkPrefetch == right.isLinkPrefetch
+#if ENABLE(APPLICATION_MANIFEST)
+        && left.isApplicationManifest == right.isApplicationManifest
+#endif
+        ;
+}
+
+inline bool operator!=(const LinkRelAttribute& left, const LinkRelAttribute& right)
+{
+    return !(left == right);
+}
+
 }