Skip to content

Commit c685d23

Browse files
yoavweissyoavweiss
committed
Speculation Rules: ensure that non-visible anchors with visible descendants get prefetched
https://bugs.webkit.org/show_bug.cgi?id=306860 Reviewed by Alex Christensen. Anchor elements with `display: contents` may not be visible but could have visible descendants. The current speculation rules logic doesn't account for that, and prevents prefetches on such elements from working. This fixes that by looking at their descendants and seeing if they are rendered. Test: imported/w3c/web-platform-tests/speculation-rules/prefetch/document-rules-visibility.https.html * LayoutTests/imported/w3c/web-platform-tests/speculation-rules/prefetch/document-rules-visibility.https-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/speculation-rules/prefetch/document-rules-visibility.https.html: Added. * Source/WebCore/dom/SpeculationRulesMatcher.cpp: (WebCore::hasRenderedDescendants): Check if any descendant is rendered. (WebCore::SpeculationRulesMatcher::hasMatchingRule): Expand on the visibility logic to include descendant check. Canonical link: https://commits.webkit.org/306730@main
1 parent 5c83f9e commit c685d23

File tree

3 files changed

+220
-1
lines changed

3 files changed

+220
-1
lines changed

‎LayoutTests/imported/w3c/web-platform-tests/speculation-rules/prefetch/document-rules-visibility.https-expected.txt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
2+
PASS test that link with display:contents and text content is prefetched
3+
PASS test that link with display:contents and visible child element is prefetched
4+
PASS test that customized built-in anchor element with shadow DOM is prefetched
5+
PASS test that slotted anchor inside custom element is prefetched
6+
PASS test that empty link with display:contents is NOT prefetched
7+
PASS test that link with display:contents and only hidden children is NOT prefetched
8+

‎LayoutTests/imported/w3c/web-platform-tests/speculation-rules/prefetch/document-rules-visibility.https.html‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
<!DOCTYPE html>
2+
<script src="/resources/testharness.js"></script>
3+
<script src="/resources/testharnessreport.js"></script>
4+
<script src="/common/utils.js"></script>
5+
<script src="/common/dispatcher/dispatcher.js"></script>
6+
<script src="../resources/utils.js"></script>
7+
<script src="resources/utils.sub.js"></script>
8+
9+
<body>
10+
<div id="test-container"></div>
11+
<script>
12+
setup(() => assertSpeculationRulesIsSupported());
13+
14+
// Helper to clean up after each test
15+
function cleanup(t, elements) {
16+
t.add_cleanup(() => {
17+
for (const el of elements) {
18+
el.remove();
19+
}
20+
});
21+
}
22+
23+
// Helper to add link to test container instead of body
24+
function addTestLink(url, container = document.getElementById('test-container')) {
25+
const a = document.createElement('a');
26+
a.href = url;
27+
container.appendChild(a);
28+
return a;
29+
}
30+
31+
// Helper to insert a document rule that only matches links with a specific class
32+
function insertDocumentRuleForClass(className) {
33+
const script = document.createElement('script');
34+
script.type = 'speculationrules';
35+
script.textContent = JSON.stringify({
36+
prefetch: [{
37+
source: 'document',
38+
eagerness: 'immediate',
39+
where: { selector_matches: `a.${className}` }
40+
}]
41+
});
42+
document.head.appendChild(script);
43+
return script;
44+
}
45+
46+
promise_test(async t => {
47+
// Add a link with display:contents - the link itself has no renderer
48+
// but its children do.
49+
const style = document.createElement('style');
50+
style.textContent = '.display-contents-link { display: contents; }';
51+
document.head.appendChild(style);
52+
53+
const url = getPrefetchUrl();
54+
const link = addTestLink(url);
55+
link.className = 'display-contents-link';
56+
link.textContent = 'Click me';
57+
58+
const rule = insertDocumentRuleForClass('display-contents-link');
59+
cleanup(t, [style, link, rule]);
60+
61+
await new Promise(resolve => t.step_timeout(resolve, 2000));
62+
63+
assert_equals(await isUrlPrefetched(url), 1);
64+
}, 'test that link with display:contents and text content is prefetched');
65+
66+
promise_test(async t => {
67+
// Add a link with display:contents containing a visible child element.
68+
const style = document.createElement('style');
69+
style.textContent = '.display-contents-link-2 { display: contents; }';
70+
document.head.appendChild(style);
71+
72+
const url = getPrefetchUrl();
73+
const link = addTestLink(url);
74+
link.className = 'display-contents-link-2';
75+
const span = document.createElement('span');
76+
span.textContent = 'Visible child';
77+
link.appendChild(span);
78+
79+
const rule = insertDocumentRuleForClass('display-contents-link-2');
80+
cleanup(t, [style, link, rule]);
81+
82+
await new Promise(resolve => t.step_timeout(resolve, 2000));
83+
84+
assert_equals(await isUrlPrefetched(url), 1);
85+
}, 'test that link with display:contents and visible child element is prefetched');
86+
87+
promise_test(async t => {
88+
// A custom element that renders via shadow DOM.
89+
// Note: custom elements can only be defined once, so we use a unique name per test run
90+
const customElementName = 'custom-link-' + token().substring(0, 8);
91+
class CustomLink extends HTMLAnchorElement {
92+
constructor() {
93+
super();
94+
this.attachShadow({ mode: 'open' });
95+
this.shadowRoot.innerHTML = '<span>Shadow content</span>';
96+
}
97+
}
98+
customElements.define(customElementName, CustomLink, { extends: 'a' });
99+
100+
const url = getPrefetchUrl();
101+
const link = document.createElement('a', { is: customElementName });
102+
link.href = url;
103+
link.className = 'custom-shadow-link';
104+
document.getElementById('test-container').appendChild(link);
105+
106+
const rule = insertDocumentRuleForClass('custom-shadow-link');
107+
cleanup(t, [link, rule]);
108+
109+
await new Promise(resolve => t.step_timeout(resolve, 2000));
110+
111+
assert_equals(await isUrlPrefetched(url), 1);
112+
}, 'test that customized built-in anchor element with shadow DOM is prefetched');
113+
114+
promise_test(async t => {
115+
// An autonomous custom element wrapping an anchor.
116+
// The anchor is slotted and should be prefetched.
117+
const wrapperName = 'link-wrapper-' + token().substring(0, 8);
118+
class LinkWrapper extends HTMLElement {
119+
constructor() {
120+
super();
121+
this.attachShadow({ mode: 'open' });
122+
this.shadowRoot.innerHTML = '<div class="wrapper"><slot></slot></div>';
123+
}
124+
}
125+
customElements.define(wrapperName, LinkWrapper);
126+
127+
const wrapper = document.createElement(wrapperName);
128+
document.getElementById('test-container').appendChild(wrapper);
129+
130+
const url = getPrefetchUrl();
131+
const link = document.createElement('a');
132+
link.href = url;
133+
link.className = 'slotted-link';
134+
wrapper.appendChild(link);
135+
136+
const rule = insertDocumentRuleForClass('slotted-link');
137+
cleanup(t, [wrapper, rule]);
138+
139+
await new Promise(resolve => t.step_timeout(resolve, 2000));
140+
141+
assert_equals(await isUrlPrefetched(url), 1);
142+
}, 'test that slotted anchor inside custom element is prefetched');
143+
144+
promise_test(async t => {
145+
// Link with display:contents but no children - should NOT be prefetched
146+
// because it has no rendered descendants.
147+
const style = document.createElement('style');
148+
style.textContent = '.empty-display-contents { display: contents; }';
149+
document.head.appendChild(style);
150+
151+
const url = getPrefetchUrl();
152+
const link = addTestLink(url);
153+
link.className = 'empty-display-contents';
154+
// No text content or children
155+
156+
const rule = insertDocumentRuleForClass('empty-display-contents');
157+
cleanup(t, [style, link, rule]);
158+
159+
await new Promise(resolve => t.step_timeout(resolve, 2000));
160+
161+
assert_equals(await isUrlPrefetched(url), 0);
162+
}, 'test that empty link with display:contents is NOT prefetched');
163+
164+
promise_test(async t => {
165+
// Link with display:contents but all children are also display:none.
166+
const style = document.createElement('style');
167+
style.textContent = `
168+
.contents-with-hidden-children { display: contents; }
169+
.contents-with-hidden-children > * { display: none; }
170+
`;
171+
document.head.appendChild(style);
172+
173+
const url = getPrefetchUrl();
174+
const link = addTestLink(url);
175+
link.className = 'contents-with-hidden-children';
176+
const span = document.createElement('span');
177+
span.textContent = 'Hidden child';
178+
link.appendChild(span);
179+
180+
const rule = insertDocumentRuleForClass('contents-with-hidden-children');
181+
cleanup(t, [style, link, rule]);
182+
183+
await new Promise(resolve => t.step_timeout(resolve, 2000));
184+
185+
assert_equals(await isUrlPrefetched(url), 0);
186+
}, 'test that link with display:contents and only hidden children is NOT prefetched');
187+
</script>
188+
</body>

‎Source/WebCore/dom/SpeculationRulesMatcher.cpp‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@
2727
#include "SpeculationRulesMatcher.h"
2828

2929
#include "CheckVisibilityOptions.h"
30+
#include "ComposedTreeIterator.h"
3031
#include "Document.h"
3132
#include "Element.h"
3233
#include "HTMLAnchorElement.h"
3334
#include "JSDOMGlobalObject.h"
3435
#include "ReferrerPolicy.h"
36+
#include "RenderElement.h"
3537
#include "ScriptController.h"
3638
#include "SelectorQuery.h"
3739
#include "ShadowRoot.h"
@@ -52,6 +54,15 @@ static bool isUnslottedElement(Element& element)
5254
return false;
5355
}
5456

57+
static bool hasRenderedDescendants(Element& element)
58+
{
59+
for (CheckedRef descendant : composedTreeDescendants(element)) {
60+
if (descendant->renderer())
61+
return true;
62+
}
63+
return false;
64+
}
65+
5566
static bool matches(const SpeculationRules::DocumentPredicate&, Document&, HTMLAnchorElement&);
5667

5768
static bool matches(const SpeculationRules::URLPatternPredicate& predicate, HTMLAnchorElement& anchor)
@@ -131,9 +142,21 @@ std::optional<PrefetchRule> SpeculationRulesMatcher::hasMatchingRule(Document& d
131142
// - It's unslotted (light DOM child of a shadow host without a slot assignment)
132143
// - It or an ancestor has display:none
133144
// - It's part of content-visibility:hidden content
134-
if (isUnslottedElement(anchor) || !anchor.checkVisibility(CheckVisibilityOptions { }))
145+
if (isUnslottedElement(anchor))
135146
return std::nullopt;
136147

148+
if (!anchor.checkVisibility(CheckVisibilityOptions { })) {
149+
// checkVisibility returns false for elements with no renderer, which includes both
150+
// display:none and display:contents.
151+
//
152+
// `display: none` elements can't have rendered descendants.
153+
if (!anchor.hasDisplayContents())
154+
return std::nullopt;
155+
// `display: contents` elements can have rendered descendants, so we need to check for them.
156+
if (!hasRenderedDescendants(anchor))
157+
return std::nullopt;
158+
}
159+
137160
const auto& url = anchor.href();
138161

139162
for (auto [node, rules] : speculationRules->prefetchRules()) {

0 commit comments

Comments
 (0)