Skip to content

GitLab

T
tangled
Commit ae2efa7d authored by David Bokan's avatar David Bokan Committed by Commit Bot
Browse files
Options

Scroll fragments when manual restoration is set 

The linked bug occurs, for both text and element fragments, because we
try to avoid scrolling to fragments when the scroll offset will be
restored by history. Manual scroll restoration is a web API that allows
the page to override the scroll restoration, we treat it separately but
want to avoid scrolling the fragment in this case too.

However, the special case for manual restoration was missing a check
that the navigation was actually a history navigation. Without this
check, an ordinary (non-history) page load that sets manual restoration
before the fragment is invoked will prevent it from scrolling.

Bug: 1147568
Change-Id: I83e1a3cc12e9f40243c42ee21c52fef2acb36993
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2533149Reviewed-by: default avatarNick Burris <nburris@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827023}
parent 355fefda
Show whitespace changes
Inline Side-by-side
Showing with 33 additions and 0 deletions
third_party/blink/renderer/core/loader/frame_loader.cc
View file @ ae2efa7d
...@@ -1387,6 +1387,7 @@ void FrameLoader::ProcessFragment(const KURL& url, ...@@ -1387,6 +1387,7 @@ void FrameLoader::ProcessFragment(const KURL& url,
// restoring the past scroll offset during a history navigation. In these // restoring the past scroll offset during a history navigation. In these
// cases we assume the scroll was restored from history (by the page). // cases we assume the scroll was restored from history (by the page).
const bool uses_manual_scroll_restoration = const bool uses_manual_scroll_restoration =
frame_load_type == WebFrameLoadType::kBackForward &&
GetDocumentLoader()->GetHistoryItem() && GetDocumentLoader()->GetHistoryItem() &&
GetDocumentLoader()->GetHistoryItem()->ScrollRestorationType() == GetDocumentLoader()->GetHistoryItem()->ScrollRestorationType() ==
mojom::blink::ScrollRestorationType::kManual; mojom::blink::ScrollRestorationType::kManual;
......
third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_test.cc
View file @ ae2efa7d
...@@ -1946,6 +1946,38 @@ TEST_F(TextFragmentAnchorTest, PageVisibility) { ...@@ -1946,6 +1946,38 @@ TEST_F(TextFragmentAnchorTest, PageVisibility) {
EXPECT_EQ(p, *GetDocument().CssTarget()); EXPECT_EQ(p, *GetDocument().CssTarget());
} }
// Regression test for https://crbug.com/1147568. Make sure a page setting
// manual scroll restoration doesn't cause the fragment to avoid scrolling on
// the initial load.
TEST_F(TextFragmentAnchorTest, ManualRestorationDoesntBlockFragment) {
SimRequest request("https://example.com/test.html#:~:text=test", "text/html");
LoadURL("https://example.com/test.html#:~:text=test");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
body {
height: 1200px;
}
p {
position: absolute;
top: 1000px;
}
</style>
<script>
history.scrollRestoration = 'manual';
</script>
<p id="text">This is a test page</p>
)HTML");
RunAsyncMatchingTasks();
// Render two frames and ensure matching and scrolling does not occur.
BeginEmptyFrame();
BeginEmptyFrame();
Element& p = *GetDocument().getElementById("text");
EXPECT_TRUE(ViewportRect().Contains(BoundingRectInFrame(p)));
}
// Test that a text directive can match across comment nodes // Test that a text directive can match across comment nodes
TEST_F(TextFragmentAnchorTest, MatchAcrossCommentNode) { TEST_F(TextFragmentAnchorTest, MatchAcrossCommentNode) {
SimRequest request("https://example.com/test.html#:~:text=abcdef", SimRequest request("https://example.com/test.html#:~:text=abcdef",
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment