Skip to content

Commit 9bea292

Browse files
basukebasuke
committed
[Navigation API] Fix scroll behavior for reload navigations with fragments.
https://bugs.webkit.org/show_bug.cgi?id=299544 rdar://161349628 Reviewed by Rupin Mittal and Chris Dumez. The NavigateEvent::processScrollBehavior() method was treating all reload navigations identically by calling restoreScrollPositionAndViewState(), which restores the previously saved scroll position. This ignored the fact that reload navigations with fragment identifiers should scroll to the fragment position instead. Tests: http/wpt/navigation-api/scroll-behavior/after-transition-reload.html http/wpt/navigation-api/scroll-behavior/manual-scroll-reload.html * LayoutTests/http/wpt/navigation-api/scroll-behavior/after-transition-reload-expected.txt: Added. * LayoutTests/http/wpt/navigation-api/scroll-behavior/after-transition-reload.html: Added. * LayoutTests/http/wpt/navigation-api/scroll-behavior/manual-scroll-reload-expected.txt: Added. * LayoutTests/http/wpt/navigation-api/scroll-behavior/manual-scroll-reload.html: Added. * Source/WebCore/page/NavigateEvent.cpp: (WebCore::NavigateEvent::processScrollBehavior): Canonical link: https://commits.webkit.org/300546@main
1 parent c80e52f commit 9bea292

File tree

5 files changed

+108
-2
lines changed

5 files changed

+108
-2
lines changed

‎LayoutTests/http/wpt/navigation-api/scroll-behavior/after-transition-reload-expected.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
PASS scroll: after-transition should work on a reload navigation
3+

‎LayoutTests/http/wpt/navigation-api/scroll-behavior/after-transition-reload.html‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<!doctype html>
2+
<script src="/resources/testharness.js"></script>
3+
<script src="/resources/testharnessreport.js"></script>
4+
<body>
5+
<div id="buffer" style="height: 1000px; width: 1000px;"></div>
6+
<div id="frag"></div>
7+
<div style="height: 1000px; width: 1000px;"></div>
8+
<script>
9+
promise_test(async t => {
10+
// Wait for after the load event so that the navigation doesn't get converted
11+
// into a replace navigation.
12+
await new Promise(resolve => window.onload = () => t.step_timeout(resolve, 0));
13+
assert_equals(window.scrollY, 0);
14+
await navigation.navigate("#frag").finished;
15+
// Scroll down 10px from #frag
16+
window.scrollBy(0, 10);
17+
let scrollY_frag_plus_10 = window.scrollY;
18+
19+
let intercept_resolve;
20+
let navigate_event;
21+
navigation.onnavigate = e => {
22+
navigate_event = e;
23+
e.intercept({ handler: () => new Promise(r => intercept_resolve = r),
24+
scroll: "after-transition" });
25+
};
26+
let reload_promises = navigation.reload();
27+
await reload_promises.committed;
28+
29+
// Removing the <div id="buffer"> should scroll up 1000px if scroll anchoring is enabled.
30+
assert_equals(window.scrollY, scrollY_frag_plus_10);
31+
buffer.remove();
32+
let scrollY_after_buffer_remove = window.scrollY;
33+
// Don't assert the exact value after buffer removal since it depends on scroll anchoring.
34+
35+
// Finishing should restore scroll to #frag's position
36+
intercept_resolve();
37+
await reload_promises.finished;
38+
assert_not_equals(window.scrollY, scrollY_after_buffer_remove);
39+
40+
// After restoration, we should be at #frag's position
41+
// The scroll should be restored to make #frag visible at the same viewport position
42+
let frag_element = document.getElementById("frag");
43+
let frag_top = frag_element.offsetTop;
44+
assert_equals(window.scrollY, frag_top);
45+
}, "scroll: after-transition should work on a reload navigation");
46+
</script>
47+
</body>

‎LayoutTests/http/wpt/navigation-api/scroll-behavior/manual-scroll-reload-expected.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
PASS scroll: scroll() should work on a reload navigation
3+

‎LayoutTests/http/wpt/navigation-api/scroll-behavior/manual-scroll-reload.html‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<!doctype html>
2+
<script src="/resources/testharness.js"></script>
3+
<script src="/resources/testharnessreport.js"></script>
4+
<body>
5+
<div id="buffer" style="height: 1000px; width: 1000px;"></div>
6+
<div id="frag"></div>
7+
<div style="height: 1000px; width: 1000px;"></div>
8+
<script>
9+
promise_test(async t => {
10+
// Wait for after the load event so that the navigation doesn't get converted
11+
// into a replace navigation.
12+
await new Promise(resolve => window.onload = () => t.step_timeout(resolve, 0));
13+
assert_equals(window.scrollY, 0);
14+
await navigation.navigate("#frag").finished;
15+
// Scroll down 10px from #frag
16+
window.scrollBy(0, 10);
17+
let scrollY_frag_plus_10 = window.scrollY;
18+
19+
let intercept_resolve;
20+
let navigate_event;
21+
navigation.onnavigate = e => {
22+
navigate_event = e;
23+
e.intercept({ handler: () => new Promise(r => intercept_resolve = r),
24+
scroll: "manual" });
25+
};
26+
let reload_promises = navigation.reload();
27+
await reload_promises.committed;
28+
29+
// Removing the <div id="buffer"> should scroll up 1000px if scroll anchoring is enabled.
30+
assert_equals(window.scrollY, scrollY_frag_plus_10);
31+
buffer.remove();
32+
let scrollY_after_buffer_remove = window.scrollY;
33+
// Don't assert the exact value after buffer removal since it depends on scroll anchoring.
34+
35+
// After restoration, we should be at #frag's position
36+
// The scroll should be restored to make #frag visible at the same viewport position
37+
navigate_event.scroll();
38+
let frag_element = document.getElementById("frag");
39+
let frag_top = frag_element.offsetTop;
40+
assert_equals(window.scrollY, frag_top);
41+
assert_not_equals(window.scrollY, scrollY_after_buffer_remove);
42+
43+
// Finishing should not scroll.
44+
intercept_resolve();
45+
await reload_promises.finished;
46+
assert_equals(window.scrollY, frag_top);
47+
}, "scroll: scroll() should work on a reload navigation");
48+
</script>
49+
</body>

‎Source/WebCore/page/NavigateEvent.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,13 @@ void NavigateEvent::processScrollBehavior(Document& document)
125125
ASSERT(m_interceptionState == InterceptionState::Committed);
126126
m_interceptionState = InterceptionState::Scrolled;
127127

128-
if (m_navigationType == NavigationNavigationType::Traverse || m_navigationType == NavigationNavigationType::Reload)
128+
if (m_navigationType == NavigationNavigationType::Traverse || m_navigationType == NavigationNavigationType::Reload) {
129+
if (m_navigationType == NavigationNavigationType::Reload && document.url().hasFragmentIdentifier()) {
130+
if (document.frame()->view()->scrollToFragment(document.url()))
131+
return;
132+
}
129133
document.frame()->loader().history().restoreScrollPositionAndViewState();
130-
else if (!document.frame()->view()->scrollToFragment(document.url())) {
134+
} else if (!document.frame()->view()->scrollToFragment(document.url())) {
131135
if (!document.url().hasFragmentIdentifier())
132136
document.frame()->view()->scrollTo({ 0, 0 });
133137
}

0 commit comments

Comments
 (0)