Skip to content

GitLab

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

Fix text fragment after history.replaceState 

Text fragments are allowed only if a text fragment token has been
generated during loading. The token is effectively a navigation-time
user activation flag. To allow user initiated text fragment navigations
during a same document navigation, a token may be generated in
DocumentLoader::UpdateForSameDocumentNavigation

However, this method is called in a variety of other scenarios, such as
calls to history.replaceState or portal navigations. In those cases, we
would try to generate a new token, fail, and then clobber an existing
token, causing pages like in the linked bug to fail.

This CL makes it so that we only replace an existing token if the
navigation is a standard navigation.

Bug: 1147453
Change-Id: I2bad963884068d2de23833ffce37086fda9a034e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2533688Reviewed-by: default avatarNick Burris <nburris@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827036}
parent 70114c68
Hide whitespace changes
Inline Side-by-side
Showing with 39 additions and 5 deletions
third_party/blink/renderer/core/loader/document_loader.cc
View file @ 5ca5d0f8
......@@ -498,11 +498,14 @@ void DocumentLoader::UpdateForSameDocumentNavigation(
redirect_chain_.push_back(new_url);
// We want to allow same-document text fragment navigations if they're coming
// from the browser.
has_text_fragment_token_ =
TextFragmentAnchor::GenerateNewTokenForSameDocument(
new_url.FragmentIdentifier(), type, is_content_initiated,
same_document_navigation_source);
// from the browser. Do this only on a standard navigation so that we don't
// clobber the token when this is called from e.g. history.replaceState.
if (type == WebFrameLoadType::kStandard) {
has_text_fragment_token_ =
TextFragmentAnchor::GenerateNewTokenForSameDocument(
new_url.FragmentIdentifier(), type, is_content_initiated,
same_document_navigation_source);
}
SetHistoryItemStateForCommit(
history_item_.Get(), type,
......
third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_test.cc
View file @ 5ca5d0f8
......@@ -1978,6 +1978,37 @@ TEST_F(TextFragmentAnchorTest, ManualRestorationDoesntBlockFragment) {
EXPECT_TRUE(ViewportRect().Contains(BoundingRectInFrame(p)));
}
// Regression test for https://crbug.com/1147453. Ensure replaceState doesn't
// clobber the text fragment token and allows fragment to scroll.
TEST_F(TextFragmentAnchorTest, ReplaceStateDoesntBlockFragment) {
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.replaceState({}, 'test', '');
</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_F(TextFragmentAnchorTest, MatchAcrossCommentNode) {
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