Bug 216979 – Fix image-loading-lazy-multiple-times.html

• Component: Images
• Status: REOPENED

[Rob Buis]

Fix image-loading-lazy-multiple-times.html

[Rob Buis]

Created attachment 409703 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/69885316>

[Rob Buis]

Created attachment 410771 [details]
Patch

[Rob Buis]

Created attachment 410810 [details]
Patch

[EWS]

Committed r268178: <https://trac.webkit.org/changeset/268178>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 410810 [details].

[Karl Rackler]

Reverted r268178 for reason:

This reverts commit r268178 because it caused a test failure.

Committed r268619: <https://trac.webkit.org/changeset/268619>

Comment on attachment 410810 [details]
Patch

>Subversion Revision: 268138
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index b79f977536dae32756d2f6d54e2beede7f28991e..d90086924a8b0add06e14ee683c99b77924916c0 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,18 @@
>[phone]  Rob Buis  <[e-mail]>
>+
>+        Fix image-loading-lazy-multiple-times.html
>+        https://bugs.webkit.org/show_bug.cgi?id=216979
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Once an image has been lazy loaded, it should be possible
>+        to trigger a new lazy load through relevant mutations.
>+
>+        Test: imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-multiple-times.html
>+
>+        * loader/ImageLoader.cpp:
>+        (WebCore::ImageLoader::updateFromElement):
>+
> 2020-10-07  Noam Rosenthal  <[e-mail]>
> 
>         clip-path: path() ignores page zooming (Command-+)
>diff --git a/Source/WebCore/loader/ImageLoader.cpp b/Source/WebCore/loader/ImageLoader.cpp
>index 6c065d7813b194ee95f61b69041e1401bdb3ce0f..4899c2777981bd84f0a7433cb9d7f667bbe3959d 100644
>--- a/Source/WebCore/loader/ImageLoader.cpp
>+++ b/Source/WebCore/loader/ImageLoader.cpp
>@@ -206,7 +206,7 @@ void ImageLoader::updateFromElement(RelevantMutation relevantMutation)
>             document.cachedResourceLoader().m_documentResources.set(newImage->url().string(), newImage.get());
>             document.cachedResourceLoader().setAutoLoadImages(autoLoadOtherImages);
>         } else {
>-            if (m_lazyImageLoadState == LazyImageLoadState::None && isImageElement) {
>+            if ((m_lazyImageLoadState == LazyImageLoadState::None || m_lazyImageLoadState == LazyImageLoadState::FullImage) && isImageElement) {
>                 auto& imageElement = downcast<HTMLImageElement>(element());
>                 if (imageElement.isLazyLoadable() && document.settings().lazyImageLoadingEnabled()) {
>                     m_lazyImageLoadState = LazyImageLoadState::Deferred;
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index b62ef007ce4d1e92ec6e108daac7983d6e85d44e..1c6cc4b8816972ecdf289a075766cc87806e7e67 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,14 @@
>[phone]  Rob Buis  <[e-mail]>
>+
>+        Fix image-loading-lazy-multiple-times.html
>+        https://bugs.webkit.org/show_bug.cgi?id=216979
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add improved test result.
>+
>+        * web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-multiple-times-expected.txt:
>+
> 2020-10-07  Noam Rosenthal  <[e-mail]>
> 
>         clip-path: path() ignores page zooming (Command-+)
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-multiple-times-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-multiple-times-expected.txt
>index 33d738a925ea916824b7d5c00d4879db48be469c..830329d72649758bae5dd4c2b8e11f8d7e4136f2 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-multiple-times-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-multiple-times-expected.txt
>@@ -1,4 +1,4 @@
> 
> 
>-FAIL Images with loading='lazy' can be lazy loaded multiple times assert_unreached: The loading=lazy below-viewport image should lazily load its second image, and not load it eagerly when the `src` attribute is changed Reached unreachable code
>+PASS Images with loading='lazy' can be lazy loaded multiple times
>

[Rob Buis]

Created attachment 423482 [details]
Patch

[Rob Buis]

Created attachment 441601 [details]
Patch

[Rob Buis]

Created attachment 447444 [details]
Patch

[Rob Buis]

Created attachment 448088 [details]
Patch

[Rob Buis]

Created attachment 462073 [details]
Patch

[Rob Buis]

Created attachment 462092 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Rob Buis]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/35766

[EWS]

Committed 254127@main (fe2b4d3a37eb): <https://commits.webkit.org/254127@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 462092 [details].

[WebKit Commit Bot]

Re-opened since this is blocked by bug 247297

[Said Abou-Hallawa]

The change 254127@main causes the images on nytimes.com to flicker when hovering.

[Darin Adler]

To be clear: This change *Was* causing that, and is now reverted.

[Karl Dubost]

See the discussions in Bug 248025
and https://github.com/whatwg/html/issues/8524

I wonder if there is ambiguity in between the two.