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
: 232691 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-10-26 09:12 PDT by Chris Dumez
Modified: 2022-07-28 23:44 PDT (History)
15 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.