Skip to content

Commit ca1bcbb

Browse files
rwlbuisasurdej-comcast
authored andcommitted
Images with loading="lazy" have uncontrollable gray border while loading https://bugs.webkit.org/show_bug.cgi?id=243601
Reviewed by Darin Adler. Do not paint border while an image is in deferred state. The test image-loading-lazy-slow.html covers this. However, the current test runner logic stops page loads before making a pixel snapshot, causing the image to be painted as a broken image instead of the empty image at the time of calling takeScreenshot. To fix this, postpone the stopping of page loads and instead always stop page loads when reseting after the test. Note that printing tests are not affected since they already made a pixel snapshot before stopping the page loads. * LayoutTests/TestExpectations: * LayoutTests/platform/ios/TestExpectations: * LayoutTests/platform/mac-wk1/TestExpectations: * Source/WebCore/rendering/RenderImage.cpp: (WebCore::RenderImage::paintReplaced): * Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp: (WTR::InjectedBundle::didReceiveMessageToPage): (WTR::InjectedBundle::done): Canonical link: https://commits.webkit.org/253960@main
1 parent b13b132 commit ca1bcbb

3 files changed

Lines changed: 11 additions & 4 deletions

File tree

LayoutTests/TestExpectations

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,6 @@ imported/w3c/web-platform-tests/html/rendering/widgets/the-select-element/option
783783
imported/w3c/web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe-modify-scrolling-attr-to-yes.html [ ImageOnlyFailure ]
784784
imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/adopt-from-image-document.html [ ImageOnlyFailure ]
785785
imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-slow-aspect-ratio.html [ ImageOnlyFailure ]
786-
imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-slow.html [ ImageOnlyFailure ]
787786
imported/w3c/web-platform-tests/html/semantics/forms/the-option-element/dynamic-content-change-rendering.html [ ImageOnlyFailure ]
788787
imported/w3c/web-platform-tests/html/semantics/forms/the-selectmenu-element/selectmenu-option-arbitrary-content-displayed.tentative.html [ ImageOnlyFailure ]
789788
imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-popup-element/popup-hidden-display.tentative.html [ ImageOnlyFailure ]

Source/WebCore/rendering/RenderImage.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -479,14 +479,17 @@ void RenderImage::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOf
479479
float deviceScaleFactor = document().deviceScaleFactor();
480480
LayoutUnit missingImageBorderWidth(1 / deviceScaleFactor);
481481

482+
if (isDeferredImage(element()))
483+
return;
484+
482485
if (context.detectingContentfulPaint()) {
483-
if (!context.contenfulPaintDetected() && !isDeferredImage(element()) && cachedImage() && cachedImage()->canRender(this, deviceScaleFactor) && !contentSize.isEmpty())
486+
if (!context.contenfulPaintDetected() && cachedImage() && cachedImage()->canRender(this, deviceScaleFactor) && !contentSize.isEmpty())
484487
context.setContentfulPaintDetected();
485488

486489
return;
487490
}
488491

489-
if (!imageResource().cachedImage() || isDeferredImage(element()) || shouldDisplayBrokenImageIcon()) {
492+
if (!imageResource().cachedImage() || shouldDisplayBrokenImageIcon()) {
490493
if (paintInfo.phase == PaintPhase::Selection)
491494
return;
492495

Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,8 @@ void InjectedBundle::didReceiveMessageToPage(WKBundlePageRef page, WKStringRef m
222222
m_state = Idle;
223223
m_dumpPixels = false;
224224
m_pixelResultIsPending = false;
225+
// Needed for pixel result pending mode, otherwise a no-op.
226+
InjectedBundle::page()->stopLoading();
225227

226228
setlocale(LC_ALL, "");
227229
TestRunner::removeAllWebNotificationPermissions();
@@ -552,7 +554,10 @@ void InjectedBundle::done()
552554

553555
m_useWorkQueue = false;
554556

555-
page()->stopLoading();
557+
// Postpone page load stop if pixel result is still pending since
558+
// cancelled image loads will paint as broken images.
559+
if (!m_pixelResultIsPending)
560+
page()->stopLoading();
556561
setTopLoadingFrame(0);
557562

558563
#if ENABLE(ACCESSIBILITY)

0 commit comments

Comments
 (0)