Commit 564d7d30 authored by Nick Burris's avatar Nick Burris Committed by Commit Bot

Implement :~: fragment directive delimiter

Implement the newly proposed fragment directive delimiter :~:. This
delimiter behaves the same as the previously implemented ## delimiter,
but is valid by URL spec and thus should be a better option. For now,
both delimiters are supported, but the ## delimiter will be removed for
M79 once we confirm we want to use :~:.

Bug: 1007016
Change-Id: Icd732d27ccc3a7591617735afee9d14d5d1c68f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1822512
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699589}
parent f1417a17
...@@ -4533,26 +4533,43 @@ void Document::SetURL(const KURL& url) { ...@@ -4533,26 +4533,43 @@ void Document::SetURL(const KURL& url) {
// If text fragment identifiers are enabled, we strip the fragment directive // If text fragment identifiers are enabled, we strip the fragment directive
// from the URL fragment. // from the URL fragment.
// E.g. "#id##targetText=a" --> "#id" // E.g. "#id:~:targetText=a" --> "#id"
if (RuntimeEnabledFeatures::TextFragmentIdentifiersEnabled(this)) { if (RuntimeEnabledFeatures::TextFragmentIdentifiersEnabled(this)) {
// Add a hash to the beginning of the fragment as it is part of parsing the String fragment = new_url.FragmentIdentifier();
// ## fragment directive prefix but is not included in FragmentIdentifier(). wtf_size_t start_pos = fragment.Find(kFragmentDirectiveNewPrefix);
String fragment = "#" + new_url.FragmentIdentifier(); if (start_pos != kNotFound) {
fragment_directive_ = fragment.Substring(
start_pos + kFragmentDirectiveNewPrefixStringLength);
if (start_pos == 0)
new_url.RemoveFragmentIdentifier();
else
new_url.SetFragmentIdentifier(fragment.Substring(0, start_pos));
} else {
// TODO(crbug/1007016): Remove support for the old prefix, we currently
// fall back to checking ## if we did not find the new delimiter :~:.
// Add a hash to the beginning of the fragment as it is part of parsing
// the ## fragment directive prefix but is not included in
// FragmentIdentifier().
fragment = "#" + fragment;
wtf_size_t start_pos = fragment.Find(kFragmentDirectivePrefix); wtf_size_t start_pos = fragment.Find(kFragmentDirectivePrefix);
if (start_pos != kNotFound) { if (start_pos != kNotFound) {
fragment_directive_ = fragment_directive_ = fragment.Substring(
fragment.Substring(start_pos + kFragmentDirectivePrefixStringLength); start_pos + kFragmentDirectivePrefixStringLength);
if (start_pos == 0) { if (start_pos == 0) {
// Remove the URL fragment if it is entirely a fragment directive. // Remove the URL fragment if it is entirely a fragment directive.
new_url.RemoveFragmentIdentifier(); new_url.RemoveFragmentIdentifier();
} else { } else {
// The stripped fragment starts at position 1 and has length start_pos-1 // The stripped fragment starts at position 1 and has length
// due to the added hash. // start_pos-1 due to the added hash.
String stripped_fragment = fragment.Substring(1, start_pos - 1); String stripped_fragment = fragment.Substring(1, start_pos - 1);
new_url.SetFragmentIdentifier(stripped_fragment); new_url.SetFragmentIdentifier(stripped_fragment);
} }
} }
} }
}
url_ = new_url; url_ = new_url;
access_entry_from_url_ = nullptr; access_entry_from_url_ = nullptr;
......
...@@ -24,6 +24,12 @@ constexpr char kFragmentDirectivePrefix[] = "##"; ...@@ -24,6 +24,12 @@ constexpr char kFragmentDirectivePrefix[] = "##";
// Subtract 1 because base::size includes the \0 string terminator. // Subtract 1 because base::size includes the \0 string terminator.
constexpr size_t kFragmentDirectivePrefixStringLength = constexpr size_t kFragmentDirectivePrefixStringLength =
base::size(kFragmentDirectivePrefix) - 1; base::size(kFragmentDirectivePrefix) - 1;
// TODO(crbug/1007016): Remove support for the old prefix once we confirm the
// new prefix choice.
constexpr char kFragmentDirectiveNewPrefix[] = ":~:";
// Subtract 1 because base::size includes the \0 string terminator.
constexpr size_t kFragmentDirectiveNewPrefixStringLength =
base::size(kFragmentDirectiveNewPrefix) - 1;
enum class TextFragmentFormat { PlainFragment, FragmentDirective }; enum class TextFragmentFormat { PlainFragment, FragmentDirective };
......
...@@ -1562,6 +1562,105 @@ TEST_F(TextFragmentAnchorTest, DismissTextHighlightInView) { ...@@ -1562,6 +1562,105 @@ TEST_F(TextFragmentAnchorTest, DismissTextHighlightInView) {
EXPECT_FALSE(GetDocument().View()->GetFragmentAnchor()); EXPECT_FALSE(GetDocument().View()->GetFragmentAnchor());
} }
// Test that the fragment directive delimiter :~: works properly and is stripped
// from the URL.
TEST_F(TextFragmentAnchorTest, FragmentDirectiveDelimiter) {
SimRequest request("https://example.com/test.html#:~:targetText=test",
"text/html");
LoadURL("https://example.com/test.html#:~:targetText=test");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
body {
height: 1200px;
}
p {
position: absolute;
top: 1000px;
}
</style>
<p id="text">This is a test page</p>
)HTML");
Compositor().BeginFrame();
RunAsyncMatchingTasks();
EXPECT_EQ(1u, GetDocument().Markers().Markers().size());
EXPECT_EQ(GetDocument().Url(), "https://example.com/test.html");
}
// Test that a :~: fragment directive is scrolled into view and is stripped from
// the URL when there's also a valid element fragment.
TEST_F(TextFragmentAnchorTest, FragmentDirectiveDelimiterWithElementFragment) {
SimRequest request("https://example.com/test.html#element:~:targetText=test",
"text/html");
LoadURL("https://example.com/test.html#element:~:targetText=test");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
body {
height: 2200px;
}
#text {
position: absolute;
top: 1000px;
}
#element {
position: absolute;
top: 2000px;
}
</style>
<p id="text">This is a test page</p>
<div id="element">Some text</div>
)HTML");
Compositor().BeginFrame();
RunAsyncMatchingTasks();
EXPECT_EQ(GetDocument().Url(), "https://example.com/test.html#element");
Element& p = *GetDocument().getElementById("text");
EXPECT_TRUE(ViewportRect().Contains(BoundingRectInFrame(p)))
<< "<p> Element wasn't scrolled into view, viewport's scroll offset: "
<< LayoutViewport()->GetScrollOffset().ToString();
}
// Test that a fragment directive is stripped from the URL even if it is not a
// targetText.
TEST_F(TextFragmentAnchorTest, IdFragmentWithFragmentDirective) {
SimRequest request("https://example.com/test.html#element:~:id", "text/html");
LoadURL("https://example.com/test.html#element:~:id");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
body {
height: 2200px;
}
p {
position: absolute;
top: 1000px;
}
div {
position: absolute;
top: 2000px;
}
</style>
<p id="element">This is a test page</p>
<div id="element:~:id">Some text</div>
)HTML");
Compositor().BeginFrame();
RunAsyncMatchingTasks();
Element& div = *GetDocument().getElementById("element");
EXPECT_TRUE(ViewportRect().Contains(BoundingRectInFrame(div)))
<< "Should have scrolled <div> into view but didn't, scroll offset: "
<< LayoutViewport()->GetScrollOffset().ToString();
}
} // namespace } // namespace
} // namespace blink } // namespace blink
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