Bug 232309 - link elements should be able to fire more than one load / error event
Summary: link elements should be able to fire more than one load / error event
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-26 09:12 PDT by Chris Dumez
Modified: 2022-09-13 16:57 PDT (History)
16 users (show)

See Also:


Attachments
Patch (7.19 KB, patch)
2021-10-26 09:14 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2021-10-26 09:12:54 PDT
<link> elements should be able to fire more than one load / error event.
Comment 1 Chris Dumez 2021-10-26 09:14:57 PDT
Created attachment 442494 [details]
Patch
Comment 2 Darin Adler 2021-10-26 11:03:12 PDT
Comment on attachment 442494 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=442494&action=review

> Source/WebCore/ChangeLog:10
> +        We had logic to only fire a single load / error event for <link> elements, even
> +        though they could do several loads. This logic is not part of the specification
> +        and was causing us to fail some WPT tests.

I remember that such booleans, long ago, were used to work around various bugs. However that’s a really old memory, more like 15 years ago. I hope this is no longer the case, and this doesn’t re-expose a bug in some case.
Comment 3 EWS 2021-10-26 11:26:58 PDT
Committed r284883 (243560@main): <https://commits.webkit.org/243560@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 442494 [details].
Comment 4 Radar WebKit Bug Importer 2021-10-26 11:27:21 PDT
<rdar://problem/84672716>
Comment 5 Fujii Hironori 2021-11-03 19:19:22 PDT
I have a problem for this change.
 Bug 232691 – REGRESSION(r284883): load event of link element is fired repeatedly on https://cmake.org/
Comment 6 Chris Dumez 2021-11-08 11:15:21 PST
Reverted r284883 for reason:

Caused high energy use on wsj.com <rdar://85156874>

Committed r285417 (243974@main): <https://commits.webkit.org/243974@main>
Comment 7 Fujii Hironori 2021-11-08 11:59:16 PST
*** Bug 232691 has been marked as a duplicate of this bug. ***
Comment 8 Ahmad Saleem 2022-07-26 10:42:47 PDT
I am not sure on web-spec but it seems Safari and Chrome are still failing these tests:

https://wpt.fyi/results/html/semantics/document-metadata/the-link-element?label=master&label=experimental&aligned&q=link-multiple

Just wanted to share updated results. Thanks!
Comment 9 Ryosuke Niwa 2022-07-26 21:55:06 PDT
Maybe we need to modify the spec to mandate this behavior if wsj.com and other websites are relying on the current behavior of WebKit.
Comment 10 Fujii Hironori 2022-07-27 14:39:45 PDT
https://www.wsj.com/ has the code like this:

> <link href="https://video-api.wsj.com/api-video/audio/css/audioplayer.min.css" rel="preload" as="style" onload="this.rel='stylesheet'">

I don't know why changing 'rel' attribute should fire infinite load events.
Comment 11 Darin Adler 2022-07-27 17:57:01 PDT
Given that, it looks like we might have avoided the high energy use at wsj.com if changing the value of the 'rel' attribute caused a new load only if the value is a different one. I wonder if it’s a mistake that re-setting the 'rel' attribute to the same value triggers a load.
Comment 12 Ryosuke Niwa 2022-07-27 21:13:54 PDT
Gecko & Chrome don't seem to reload when setting rel to stylesheet (same value) so that might be the fix.
Comment 13 Ryosuke Niwa 2022-07-27 23:19:46 PDT
Pull request: https://github.com/WebKit/WebKit/pull/2805
Comment 14 Ryosuke Niwa 2022-07-28 20:11:05 PDT
Filed https://github.com/whatwg/html/issues/8138 to track the issue with the HTML spec.
Comment 15 EWS 2022-07-28 23:44:37 PDT
Committed 252943@main (a697ed80d982): <https://commits.webkit.org/252943@main>

Reviewed commits have been landed. Closing PR #2805 and removing active labels.
Comment 16 Alexey Proskuryakov 2022-09-07 07:30:22 PDT
This was reverted again, https://commits.webkit.org/254213@main
Comment 17 Ryosuke Niwa 2022-09-07 22:34:00 PDT
aa.com has this markup:

<link rel="stylesheet" href="/booking/resources/styles.6eed4fecbbccb98e596b.css" media="all" onload="this.media='all'">

cbsnews.com also has this markup:

<link rel="stylesheet" href="/fly/bundles/cbsnewscontent/css/responsive.min.css?v=8bc2e0f78408a4b8086da85d60d8c0a5" media="print" onload="this.media='all'">

I guess this is some kind of a pattern used by some framework / library.
Comment 18 Ryosuke Niwa 2022-09-08 00:45:49 PDT
Pull request: https://github.com/WebKit/WebKit/pull/4123
Comment 19 Ahmad Saleem 2022-09-08 14:45:50 PDT
(In reply to Ryosuke Niwa from comment #17)
> aa.com has this markup:
> 
> <link rel="stylesheet"
> href="/booking/resources/styles.6eed4fecbbccb98e596b.css" media="all"
> onload="this.media='all'">
> 
> cbsnews.com also has this markup:
> 
> <link rel="stylesheet"
> href="/fly/bundles/cbsnewscontent/css/responsive.min.
> css?v=8bc2e0f78408a4b8086da85d60d8c0a5" media="print"
> onload="this.media='all'">
> 
> I guess this is some kind of a pattern used by some framework / library.

I raised similar bug where the website was flashing and it has similar "onload..." as Bug 244154.

If Bug 244154 is indeed because of this bug, we can mark the other as "DUPLICATE OF" bug. Just wanted to confirm. Thanks!
Comment 20 EWS 2022-09-08 21:49:55 PDT
Committed 254290@main (3775c2edb230): <https://commits.webkit.org/254290@main>

Reviewed commits have been landed. Closing PR #4123 and removing active labels.
Comment 21 Sam Sneddon [:gsnedders] 2022-09-13 16:57:46 PDT
*** Bug 245133 has been marked as a duplicate of this bug. ***