Native image Lazyloading sometimes does not load

https://bugs.webkit.org/show_bug.cgi?id=237703 Reviewed by Darin Adler. Fix the problem that data URL placeholders are not replaced with the target image by keeping track of pending URL loads ; if the new request is done with the same URL we keep the original URL since it contains the original base URL, otherwise we use the new URL. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-crossorigin-change.sub.html: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-data-url-to-https-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-data-url-to-https-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-data-url-to-https.html: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-empty-src-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-empty-src.html: Added. * LayoutTests/platform/mac-wk1/TestExpectations: * Source/WebCore/loader/ImageLoader.cpp: (WebCore::ImageLoader::updateFromElement): (WebCore::ImageLoader::notifyFinished): * Source/WebCore/loader/ImageLoader.h: Canonical link: https://commits.webkit.org/254471@main
WebKit · Sep 14, 2022 · 0c40ba62b482511fe03646f1d4982efd727475dd · 0c40ba6
1 parent ce99e01
commit 0c40ba62b482511fe03646f1d4982efd727475dd
Show file tree

Hide file tree

Showing 9 changed files with 70 additions and 4 deletions.
diff --git a/...semantics/embedded-content/the-img-element/image-loading-lazy-crossorigin-change.sub.html b/...semantics/embedded-content/the-img-element/image-loading-lazy-crossorigin-change.sub.html
@@ -28,7 +28,7 @@
 
     img.promise
       .then(t.unreached_func("The image should not load."))
-      .catch(t.step_func_done());
+      .catch(t.step_func(() => { img.element().onload = t.step_func_done(); img.element().src = 'resources/image.png'; }));
   }, "Test that when deferred image is loaded, it uses the latest crossorigin attribute.");
 </script>
 

diff --git a/...ntics/embedded-content/the-img-element/image-loading-lazy-data-url-to-https-expected.html b/...ntics/embedded-content/the-img-element/image-loading-lazy-data-url-to-https-expected.html
@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<img src="resources/image.png">
diff --git a/.../semantics/embedded-content/the-img-element/image-loading-lazy-data-url-to-https-ref.html b/.../semantics/embedded-content/the-img-element/image-loading-lazy-data-url-to-https-ref.html
@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<img src="resources/image.png">
diff --git a/...html/semantics/embedded-content/the-img-element/image-loading-lazy-data-url-to-https.html b/...html/semantics/embedded-content/the-img-element/image-loading-lazy-data-url-to-https.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+  <title>Lazy loaded Images with data url placeholders can be overwritten by a src change</title>
+  <link rel="help" href="https://html.spec.whatwg.org/multipage/images.html#update-the-image-data">
+  <link rel="match" href="image-loading-lazy-data-url-to-https-ref.html">
+  <script src="/common/reftest-wait.js"></script>
+</head>
+
+<body>
+  <img id="image" loading="lazy" src="data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 872 490' width='872' height='490' style='background: green' %3E%3C/svg%3E">
+
+<script>
+  const image = document.querySelector('#image');
+
+  window.onload = function() {
+    // trigger intersection observer through forced layout.
+    image.offsetWidth;
+    image.setAttribute("src", 'resources/image.png');
+    setTimeout(() => { takeScreenshot(); }, 100);
+  };
+</script>
+</body>
+</html>
diff --git a/...html/semantics/embedded-content/the-img-element/image-loading-lazy-empty-src-expected.txt b/...html/semantics/embedded-content/the-img-element/image-loading-lazy-empty-src-expected.txt
@@ -0,0 +1,3 @@
+
+PASS lazy loaded image and empty src
+
diff --git a/...m-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-empty-src.html b/...m-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-empty-src.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<title>Lazy loaded Images handle correctly when setting src to empty</title>
+<link rel="help" href="https://html.spec.whatwg.org/multipage/images.html#update-the-image-data">
+<div id=log></div>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<img id="image" loading="lazy" src="resources/image.png">
+
+<script>
+  const image = document.querySelector('#image');
+
+async_test(function(t) {
+    image.onerror = t.step_func(function(e) {
+        assert_equals(e.type, "error", "null image source check failed");
+        image.onload = t.step_func(function() {
+            t.done();
+        });
+        image.src = "resources/image.png";
+    });
+    image.src = "";
+}, "lazy loaded image and empty src");
+
+</script>
diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -420,6 +420,8 @@ imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/
 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-below-viewport-dynamic.html [ Skip ]
 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-base-url-2.html [ Skip ]
 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-base-url.html [ Skip ]
+imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-data-url-to-https.html [ Skip ]
+imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-empty-src.html [ Skip ]
 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-in-cross-origin-iframe-001.sub.html [ Skip ]
 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-in-cross-origin-iframe-002.sub.html [ Skip ]
 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-multiple-times.html [ Skip ]

diff --git a/Source/WebCore/loader/ImageLoader.cpp b/Source/WebCore/loader/ImageLoader.cpp
@@ -224,9 +224,14 @@ void ImageLoader::updateFromElement(RelevantMutation relevantMutation)
 
         auto crossOriginAttribute = element().attributeWithoutSynchronization(HTMLNames::crossoriginAttr);
 
-        // Use url from original request for lazy loads that are no longer in deferred state.
-        URL imageURL = m_lazyImageLoadState == LazyImageLoadState::LoadImmediately
-            ? m_image->url() : document.completeURL(sourceURI(attr));
+        // Use URL from original request for same URL loads in order to preserve the original base URL.
+        URL imageURL;
+        if (m_image && attr == m_pendingURL)
+            imageURL = m_image->url();
+        else {
+            imageURL = document.completeURL(sourceURI(attr));
+            m_pendingURL = attr;
+        }
         ResourceRequest resourceRequest(imageURL);
         resourceRequest.setInspectorInitiatorNodeIdentifier(InspectorInstrumentation::identifierForNode(m_element));
 
@@ -380,6 +385,8 @@ void ImageLoader::notifyFinished(CachedResource& resource, const NetworkLoadMetr
     ASSERT(m_failedLoadURL.isEmpty());
     ASSERT_UNUSED(resource, &resource == m_image.get());
 
+    m_pendingURL = { };
+
     if (isDeferred()) {
         LazyLoadImageObserver::unobserve(element(), element().document());
         m_lazyImageLoadState = LazyImageLoadState::FullImage;

diff --git a/Source/WebCore/loader/ImageLoader.h b/Source/WebCore/loader/ImageLoader.h
@@ -124,6 +124,7 @@ class ImageLoader : public CachedImageClient {
     Timer m_derefElementTimer;
     RefPtr<Element> m_protectedElement;
     AtomString m_failedLoadURL;
+    AtomString m_pendingURL;
     Vector<RefPtr<DeferredPromise>> m_decodingPromises;
     bool m_hasPendingBeforeLoadEvent : 1;
     bool m_hasPendingLoadEvent : 1;