Blink WPT Bot
c507a9abb6
Automatic update from web-platform-tests DOM: Fix link element processing model and attribute mutations (#53427) LinkStyle::fired_load_ exists as an over-broad, yet sometimes too-narrow bludgeon that intends to ensure a link element's load event does not fire more than once. The earliest I can trace this bit back in the Chromium code base is https://chromiumcodereview.appspot.com/2426513003 which suggests that it was inherited from WebKit. With that said, this (a) violates the specification (which has no such restriction), and (b) does not match other browsers. This restriction makes sense when trying to protect against infinite recursion caused by a link element's load event setting a link element's `href` to itself, but the restriction is over-broad in that it does not allow the load event to fire more than once, even for different resources altogether. That means if late in its lifetime, a link's `href` is changed to point to a completely different resource, the load event will not fire. At the same time, this restriction is too narrow because it only applies to `rel=stylesheet` links, and not others such as preloads. This is demonstrated by https://jsbin.com/vadotax/edit?html,output. Besides not matching the HTML Standard, this does not match other browsers. WebKit does not contain the old `m_firedLoad` parameter anymore, and it was removed in [1] / [2]. An old attempt [3] to fix this in Chromium (by me) simply removed the `fired_load_` blocker, but in https://crbug.com/41436016 we found that the case described above with infinite recursion was more common than expected (at the very least, in real-world site perf benchmarks), so the CL was reverted. This new CL attempts a more surgical restriction on the `href` setter, matching what WebKit does, and what Gecko appears to do. We want to protect against re-triggering the link processing model when the `href` attribute is "changed"/"mutated", but its value actually stays the same. (This is a weird quirk of the DOM Standard, which considers a content attribute going from its value to its same value again, as a proper attribute "change" in HTML terms). The same goes for the `rel` attribute—it only value changes should re-invoke the link loading processing model. This approach is more sensible because it: 1. Protects against the `link.href = link.href` case for *all* `rel` types, not just `rel=stylesheet` 2. It still allows the load event to fire multiple times for different resources specified by `href`—there's no reason to restrict this 3. It allows the common preload case where a link's `rel=preload` is changed to `rel=stylesheet` on load, but the `href` stays the same Finally, this CL generalizes this intervention on `href` to all attributes whose mutations trigger `Process()`. The HTML Standard should be accommodated to match what WebKit and Chromium do after this CL: https://github.com/whatwg/html/issues/11400. This is the approach WebKit does [4]. To confirm this CL has the right behavior as specified by the above list, the web platform tests in this CL assert both load event occurrences, and actual server-visible requests. [1]: https://github.com/WebKit/WebKit/pull/4123 [2]: https://github.com/WebKit/WebKit/commit/3775c2edb2307598e53c311c623d3c2d7583066c#diff-568964e1a0c5f28dee0d18f24483251642f990cdb1381312e684ac1496a506ef. [3]: https://chromium-review.googlesource.com/c/chromium/src/+/1423601 [4]: https://github.com/WebKit/WebKit/blob/b37e3c7ce24cce0b163a75013a548b7dfeb1022a/Source/WebCore/html/HTMLLinkElement.cpp#L215-L218 R=masonf Bug: 41436016, 40842661 Change-Id: I43b31128fd591683e9fc94da79035e345401d64f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6667508 Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Dominic Farolino <dom@chromium.org> Auto-Submit: Dominic Farolino <dom@chromium.org> Cr-Commit-Position: refs/heads/main@{#1479477} Co-authored-by: Dominic Farolino <dom@chromium.org> -- wpt-commits: aeff06527fa7dab01c49dc67aeb0df5dadeb981e wpt-pr: 53427
53 lines
1.9 KiB
HTML
53 lines
1.9 KiB
HTML
<!doctype html>
|
|
<meta name="timeout" content="long">
|
|
<script src="/resources/testharness.js"></script>
|
|
<script src="/resources/testharnessreport.js"></script>
|
|
<script src="/common/utils.js"></script>
|
|
<script src="/common/get-host-info.sub.js"></script>
|
|
<script src="/preload/resources/preload_helper.js"></script>
|
|
<body>
|
|
<script>
|
|
|
|
const {REMOTE_ORIGIN} = get_host_info();
|
|
|
|
function test_prefetch_change(before, after, expected, label) {
|
|
promise_test(async t => {
|
|
const link = document.createElement('link');
|
|
link.rel = 'prefetch';
|
|
t.add_cleanup(() => link.remove());
|
|
const loadErrorOrTimeout = () => new Promise(resolve => {
|
|
const timeoutMillis = 1000;
|
|
link.addEventListener('load', () => resolve('load'));
|
|
link.addEventListener('error', () => resolve('error'));
|
|
t.step_timeout(() => resolve('timeout'), timeoutMillis);
|
|
});
|
|
for (const attr in before)
|
|
link.setAttribute(attr, before[attr]);
|
|
document.head.appendChild(link);
|
|
const result1 = await loadErrorOrTimeout();
|
|
for (const attr in after) {
|
|
if (attr in before && after[attr] === null)
|
|
link.removeAttribute(attr);
|
|
else
|
|
link.setAttribute(attr, after[attr]);
|
|
}
|
|
const result2 = await loadErrorOrTimeout();
|
|
assert_array_equals([result1, result2], expected);
|
|
}, label);
|
|
}
|
|
|
|
test_prefetch_change(
|
|
{href: '/common/square.png?1'},
|
|
{href: '/common/square.png?2'},
|
|
['load', 'load'],
|
|
'Changing a prefetch href should trigger a fetch');
|
|
|
|
test_prefetch_change(
|
|
{href: `${REMOTE_ORIGIN}/common/square.png?pipe=header(Access-Control-Allow-Origin,*)`},
|
|
{href: `${REMOTE_ORIGIN}/common/square.png?pipe=header(Access-Control-Allow-Origin,*)`, crossorigin: 'anonymous'},
|
|
['load', 'timeout'],
|
|
'Changing a prefetch crossorigin attribute does not trigger a fetch');
|
|
|
|
</script>
|
|
</body>
|