Images fail to load periodically on ikea.com #6595

stwrt · 2022-11-17T18:45:50Z

`5e60d4e`

Images fail to load periodically on ikea.com
https://bugs.webkit.org/show_bug.cgi?id=248048
rdar://101657305

Reviewed by Chris Dumez.

Adding a quirk to address images showing up as grey boxes on ikea.com.
Disabling lazy image loading addresses an issue that was found
in gatsby. This issue has been addressed in a future version of gatsby.

* Source/WebCore/dom/Document.cpp:
(WebCore::Document::lazyImageLoadingEnabled const):
* Source/WebCore/dom/Document.h:
* Source/WebCore/html/parser/HTMLPreloadScanner.cpp:
(WebCore::TokenPreloadScanner::StartTagScanner::processAttribute):
* Source/WebCore/loader/ImageLoader.cpp:
(WebCore::ImageLoader::updateFromElement):
* Source/WebCore/page/Quirks.cpp:
(WebCore::Quirks::shouldExposeShowModalDialog const):
(WebCore::Quirks::shouldDisableLazyImageLoading const):
* Source/WebCore/page/Quirks.h:

Canonical link: https://commits.webkit.org/256854@main

3272fbb

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 🧪 win~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	~~🧪 api-ios~~	~~🧪 api-mac~~	✅ 🧪 api-gtk
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	~~🧪 mac-wk2~~
✅ 🛠 🧪 merge	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-11-17T18:48:03Z

EWS run on previous version of this PR (hash 18d0466)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🛠 mac-AS-debug	~~🧪 gtk-wk2~~
	✅ 🧪 api-ios	✅ 🧪 api-mac	~~🧪 api-gtk~~
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	~~🧪 mac-wk2~~
	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

cdumez

You mentioned on Slack that this patch wasn't working yet so maybe this PR should be marked as Draft?

Source/WebCore/dom/Document.cpp

cdumez · 2022-11-17T19:33:52Z

Source/WebCore/page/Quirks.cpp

@@ -1568,11 +1569,36 @@ bool Quirks::shouldExposeShowModalDialog() const
 {
    if (!needsQuirks())
        return false;
+


Unnecessary change.

Unnecessary change. This wasn't resolved.

Source/WebCore/page/Quirks.cpp

Source/WebCore/page/Quirks.h

cdumez · 2022-11-17T19:38:16Z

Images fail to load periodically on ikea.com
https://bugs.webkit.org/show_bug.cgi?id=248048

Please put radar number here too.

webkit-early-warning-system · 2022-11-17T21:31:43Z

EWS run on previous version of this PR (hash 4ab25fa)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	⏳ 🧪 api-gtk
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	~~🧪 mac-wk2~~
	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

stwrt · 2022-11-17T21:33:30Z

Images fail to load periodically on ikea.com
https://bugs.webkit.org/show_bug.cgi?id=248048

Please put radar number here too.

👍

Source/WebCore/dom/Document.cpp

Source/WebCore/page/Quirks.cpp

cdumez · 2022-11-17T22:11:20Z

Source/WebCore/page/Quirks.cpp

@@ -1568,11 +1569,36 @@ bool Quirks::shouldExposeShowModalDialog() const
 {
    if (!needsQuirks())
        return false;
+


Unnecessary change. This wasn't resolved.

Source/WebCore/page/Quirks.cpp

cdumez

There is evidence this is a recent WebKit regression. Quirks are to work around a site bug, not a WebKit bug. So why are we doing a quirk?

Also, it seems lazy image loading got enabled before the regression point?

webkit-early-warning-system · 2022-11-17T23:43:20Z

EWS run on previous version of this PR (hash a2f09c9)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

webkit-early-warning-system · 2022-11-17T23:45:33Z

Starting EWS tests for a2f09c9. Live statuses available at the PR page, #6595

webkit-early-warning-system · 2022-11-17T23:45:34Z

EWS run on previous version of this PR (hash a2f09c9)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	✅ 🧪 api-gtk
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2
	✅ 🛠 watch	❌ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

cdumez

LGTM since Brandon tells me this is not a WebKit regression (The radar probably needs updating to clarify that).

karlcow · 2022-11-18T04:57:21Z

Source/WebCore/page/Quirks.cpp


+bool Quirks::shouldDisableLazyImageLoadingQuirk() const
+{


In a local branch of mine, I have started to add comments for the quirks, because after a while it becomes very hard to understand where it comes from.

It probably be a good first step to refactor latter with messages in the console.

My format so far has been:

Suggested change

bool Quirks::shouldDisableLazyImageLoadingQuirk() const

{

// ikea.com https://bugs.webkit.org/show_bug.cgi?id=248048

bool Quirks::shouldDisableLazyImageLoadingQuirk() const

{

I was planning to add a link to the current Ikea discussion.

For the quirks I think I would like a bit more information then just a link to a bugzilla. Having information on the background of why this quirk exists, and information on how to eventually remove it would be nice too. @karlcow thoughts?

More would be better. We could come up with a kind of format where we could normalize step by step the other quirks too, but just start with this one.
Something that can be easily parsed.

webkit-early-warning-system · 2022-11-18T19:39:29Z

EWS run on current version of this PR (hash 3272fbb)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 🧪 win~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	~~🧪 api-ios~~	~~🧪 api-mac~~	✅ 🧪 api-gtk
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	~~🧪 mac-wk2~~
✅ 🛠 🧪 merge	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-11-18T19:40:01Z

EWS run on current version of this PR (hash 3272fbb)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

https://bugs.webkit.org/show_bug.cgi?id=248048 rdar://101657305 Reviewed by Chris Dumez. Adding a quirk to address images showing up as grey boxes on ikea.com. Disabling lazy image loading addresses an issue that was found in gatsby. This issue has been addressed in a future version of gatsby. * Source/WebCore/dom/Document.cpp: (WebCore::Document::lazyImageLoadingEnabled const): * Source/WebCore/dom/Document.h: * Source/WebCore/html/parser/HTMLPreloadScanner.cpp: (WebCore::TokenPreloadScanner::StartTagScanner::processAttribute): * Source/WebCore/loader/ImageLoader.cpp: (WebCore::ImageLoader::updateFromElement): * Source/WebCore/page/Quirks.cpp: (WebCore::Quirks::shouldExposeShowModalDialog const): (WebCore::Quirks::shouldDisableLazyImageLoading const): * Source/WebCore/page/Quirks.h: Canonical link: https://commits.webkit.org/256854@main

webkit-commit-queue · 2022-11-18T21:37:20Z

Committed 256854@main (5e60d4e): https://commits.webkit.org/256854@main

Reviewed commits have been landed. Closing PR #6595 and removing active labels.

stwrt requested review from cdumez and rniwa as code owners November 17, 2022 18:45

stwrt self-assigned this Nov 17, 2022

stwrt added the WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). label Nov 17, 2022

stwrt requested a review from shallawa November 17, 2022 18:46

stwrt requested review from brentfulgham and karlcow November 17, 2022 18:48

cdumez reviewed Nov 17, 2022

View reviewed changes

stwrt force-pushed the eng/ikea_quirk branch from 18d0466 to 4ab25fa Compare November 17, 2022 21:29

stwrt requested a review from cdumez November 17, 2022 21:35

cdumez reviewed Nov 17, 2022

View reviewed changes

cdumez requested changes Nov 17, 2022

View reviewed changes

stwrt force-pushed the eng/ikea_quirk branch from 4ab25fa to a2f09c9 Compare November 17, 2022 23:43

cdumez approved these changes Nov 17, 2022

View reviewed changes

karlcow reviewed Nov 18, 2022

View reviewed changes

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 18, 2022

stwrt removed the merging-blocked Applied to prevent a change from being merged label Nov 18, 2022

stwrt force-pushed the eng/ikea_quirk branch from a2f09c9 to 3272fbb Compare November 18, 2022 19:39

stwrt added the merge-queue Applied to send a pull request to merge-queue label Nov 18, 2022

webkit-early-warning-system force-pushed the eng/ikea_quirk branch from 3272fbb to 5e60d4e Compare November 18, 2022 21:37

webkit-early-warning-system merged commit 5e60d4e into WebKit:main Nov 18, 2022

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 18, 2022

stwrt deleted the eng/ikea_quirk branch November 30, 2022 18:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Images fail to load periodically on ikea.com #6595

Images fail to load periodically on ikea.com #6595

stwrt commented Nov 17, 2022 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Nov 17, 2022 •

edited

cdumez left a comment

cdumez Nov 17, 2022

cdumez Nov 17, 2022

cdumez commented Nov 17, 2022

webkit-early-warning-system commented Nov 17, 2022 •

edited

stwrt commented Nov 17, 2022

cdumez Nov 17, 2022

cdumez left a comment

webkit-early-warning-system commented Nov 17, 2022 •

edited

webkit-early-warning-system commented Nov 17, 2022

webkit-early-warning-system commented Nov 17, 2022 •

edited

cdumez left a comment

karlcow Nov 18, 2022

stwrt Nov 18, 2022

karlcow Nov 20, 2022

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-commit-queue commented Nov 18, 2022

Images fail to load periodically on ikea.com #6595

Images fail to load periodically on ikea.com #6595

Conversation

stwrt commented Nov 17, 2022 • edited by webkit-early-warning-system

5e60d4e

webkit-early-warning-system commented Nov 17, 2022 • edited

cdumez left a comment

Choose a reason for hiding this comment

cdumez Nov 17, 2022

Choose a reason for hiding this comment

cdumez Nov 17, 2022

Choose a reason for hiding this comment

cdumez commented Nov 17, 2022

webkit-early-warning-system commented Nov 17, 2022 • edited

stwrt commented Nov 17, 2022

cdumez Nov 17, 2022

Choose a reason for hiding this comment

cdumez left a comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Nov 17, 2022 • edited

webkit-early-warning-system commented Nov 17, 2022

webkit-early-warning-system commented Nov 17, 2022 • edited

cdumez left a comment

Choose a reason for hiding this comment

karlcow Nov 18, 2022

Choose a reason for hiding this comment

stwrt Nov 18, 2022

Choose a reason for hiding this comment

karlcow Nov 20, 2022

Choose a reason for hiding this comment

webkit-early-warning-system commented Nov 18, 2022 • edited

webkit-early-warning-system commented Nov 18, 2022 • edited

webkit-commit-queue commented Nov 18, 2022

stwrt commented Nov 17, 2022 •

edited by webkit-early-warning-system

`5e60d4e`

webkit-early-warning-system commented Nov 17, 2022 •

edited

webkit-early-warning-system commented Nov 17, 2022 •

edited

webkit-early-warning-system commented Nov 17, 2022 •

edited

webkit-early-warning-system commented Nov 17, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited