Summary: | Fix image-loading-lazy-multiple-times.html | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rob Buis <rbuis> | ||||||||||||||||||||
Component: | Images | Assignee: | Rob Buis <rbuis> | ||||||||||||||||||||
Status: | REOPENED --- | ||||||||||||||||||||||
Severity: | Normal | CC: | cdumez, clopez, commit-queue, darin, ews-watchlist, japhet, karlcow, rackler, sabouhallawa, simon.fraser, webkit-bug-importer, youennf | ||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=217741 https://github.com/web-platform-tests/wpt/pull/35766 https://bugs.webkit.org/show_bug.cgi?id=247279 https://bugs.webkit.org/show_bug.cgi?id=248025 |
||||||||||||||||||||||
Bug Depends on: | 247297 | ||||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||||
Attachments: |
|
Description
Rob Buis
2020-09-25 10:03:26 PDT
Created attachment 409703 [details]
Patch
Created attachment 410771 [details]
Patch
Created attachment 410810 [details]
Patch
Committed r268178: <https://trac.webkit.org/changeset/268178> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410810 [details]. 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 @@ >+2020-10-07 Rob Buis <rbuis@igalia.com> >+ >+ 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 <noam@webkit.org> > > 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 @@ >+2020-10-07 Rob Buis <rbuis@igalia.com> >+ >+ 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 <noam@webkit.org> > > 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 > Created attachment 423482 [details]
Patch
Created attachment 441601 [details]
Patch
Created attachment 447444 [details]
Patch
Created attachment 448088 [details]
Patch
Created attachment 462073 [details]
Patch
Created attachment 462092 [details]
Patch
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 Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/35766 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]. Re-opened since this is blocked by bug 247297 The change 254127@main causes the images on nytimes.com to flicker when hovering. To be clear: This change *Was* causing that, and is now reverted. See the discussions in Bug 248025 and https://github.com/whatwg/html/issues/8524 I wonder if there is ambiguity in between the two. |