Skip to content

GitLab

T
tangled
Commit f55c0034 authored by Nick Burris's avatar Nick Burris Committed by Commit Bot
Browse files
Options

Make TextFragmentAnchor matching case sensitive 

Per discussion, it makes sense for TextFragmentAnchor matching to be
case sensitive to reduce ambiguity. Case insensitivity is likely not
useful since the targetText would be generated by a copy of the text
(either by a machine or a human copy-paste).

Change-Id: I7b52d2adc4f9474fbf77bbfb45814f5ab91036c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1644835Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Commit-Queue: Nick Burris <nburris@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666455}
parent cfe6d934
Hide whitespace changes
Inline Side-by-side
Showing with 40 additions and 16 deletions
third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_test.cc
View file @ f55c0034
...@@ -397,9 +397,9 @@ TEST_F(TextFragmentAnchorTest, MultipleNonMatchingStrings) { ...@@ -397,9 +397,9 @@ TEST_F(TextFragmentAnchorTest, MultipleNonMatchingStrings) {
// Test matching a text range within the same element // Test matching a text range within the same element
TEST_F(TextFragmentAnchorTest, SameElementTextRange) { TEST_F(TextFragmentAnchorTest, SameElementTextRange) {
SimRequest request("https://example.com/test.html#targetText=this,page", SimRequest request("https://example.com/test.html#targetText=This,page",
"text/html"); "text/html");
LoadURL("https://example.com/test.html#targetText=this,page"); LoadURL("https://example.com/test.html#targetText=This,page");
request.Complete(R"HTML( request.Complete(R"HTML(
<!DOCTYPE html> <!DOCTYPE html>
<style> <style>
...@@ -619,9 +619,9 @@ TEST_F(TextFragmentAnchorTest, DistantElementTextRange) { ...@@ -619,9 +619,9 @@ TEST_F(TextFragmentAnchorTest, DistantElementTextRange) {
// Test a text range with both context terms in the same element. // Test a text range with both context terms in the same element.
TEST_F(TextFragmentAnchorTest, TextRangeWithContext) { TEST_F(TextFragmentAnchorTest, TextRangeWithContext) {
SimRequest request( SimRequest request(
"https://example.com/test.html#targetText=this-,is,test,-page", "https://example.com/test.html#targetText=This-,is,test,-page",
"text/html"); "text/html");
LoadURL("https://example.com/test.html#targetText=this-,is,test,-page"); LoadURL("https://example.com/test.html#targetText=This-,is,test,-page");
request.Complete(R"HTML( request.Complete(R"HTML(
<!DOCTYPE html> <!DOCTYPE html>
<p id="text">This is a test page</p> <p id="text">This is a test page</p>
...@@ -661,9 +661,9 @@ TEST_F(TextFragmentAnchorTest, PrefixNotFound) { ...@@ -661,9 +661,9 @@ TEST_F(TextFragmentAnchorTest, PrefixNotFound) {
// Ensure that we do not match a text range if the suffix is not found. // Ensure that we do not match a text range if the suffix is not found.
TEST_F(TextFragmentAnchorTest, SuffixNotFound) { TEST_F(TextFragmentAnchorTest, SuffixNotFound) {
SimRequest request( SimRequest request(
"https://example.com/test.html#targetText=this-,is,test,-suffix", "https://example.com/test.html#targetText=This-,is,test,-suffix",
"text/html"); "text/html");
LoadURL("https://example.com/test.html#targetText=this-,is,test,-suffix"); LoadURL("https://example.com/test.html#targetText=This-,is,test,-suffix");
request.Complete(R"HTML( request.Complete(R"HTML(
<!DOCTYPE html> <!DOCTYPE html>
<p id="text">This is a test page</p> <p id="text">This is a test page</p>
...@@ -678,11 +678,11 @@ TEST_F(TextFragmentAnchorTest, SuffixNotFound) { ...@@ -678,11 +678,11 @@ TEST_F(TextFragmentAnchorTest, SuffixNotFound) {
// Test a text range with context terms in different elements // Test a text range with context terms in different elements
TEST_F(TextFragmentAnchorTest, TextRangeWithCrossElementContext) { TEST_F(TextFragmentAnchorTest, TextRangeWithCrossElementContext) {
SimRequest request( SimRequest request(
"https://example.com/test.html#targetText=header%202-,a,text,-footer%201", "https://example.com/test.html#targetText=Header%202-,A,text,-Footer%201",
"text/html"); "text/html");
LoadURL( LoadURL(
"https://example.com/" "https://example.com/"
"test.html#targetText=header%202-,a,text,-footer%201"); "test.html#targetText=Header%202-,A,text,-Footer%201");
request.Complete(R"HTML( request.Complete(R"HTML(
<!DOCTYPE html> <!DOCTYPE html>
<h1>Header 1</h1> <h1>Header 1</h1>
...@@ -714,11 +714,11 @@ TEST_F(TextFragmentAnchorTest, TextRangeWithCrossElementContext) { ...@@ -714,11 +714,11 @@ TEST_F(TextFragmentAnchorTest, TextRangeWithCrossElementContext) {
TEST_F(TextFragmentAnchorTest, CrossElementAndWhitespaceContext) { TEST_F(TextFragmentAnchorTest, CrossElementAndWhitespaceContext) {
SimRequest request( SimRequest request(
"https://example.com/" "https://example.com/"
"test.html#targetText=list%202-,cat,-good%20cat", "test.html#targetText=List%202-,Cat,-Good%20cat",
"text/html"); "text/html");
LoadURL( LoadURL(
"https://example.com/" "https://example.com/"
"test.html#targetText=list%202-,cat,-good%20cat"); "test.html#targetText=List%202-,Cat,-Good%20cat");
request.Complete(R"HTML( request.Complete(R"HTML(
<!DOCTYPE html> <!DOCTYPE html>
<h1> List 1 </h1> <h1> List 1 </h1>
...@@ -791,9 +791,9 @@ TEST_F(TextFragmentAnchorTest, CrossEmptySiblingAndParentElementContext) { ...@@ -791,9 +791,9 @@ TEST_F(TextFragmentAnchorTest, CrossEmptySiblingAndParentElementContext) {
// Ensure we scroll to text when its prefix and suffix are out of view. // Ensure we scroll to text when its prefix and suffix are out of view.
TEST_F(TextFragmentAnchorTest, DistantElementContext) { TEST_F(TextFragmentAnchorTest, DistantElementContext) {
SimRequest request( SimRequest request(
"https://example.com/test.html#targetText=prefix-,cats,-suffix", "https://example.com/test.html#targetText=Prefix-,Cats,-Suffix",
"text/html"); "text/html");
LoadURL("https://example.com/test.html#targetText=prefix-,cats,-suffix"); LoadURL("https://example.com/test.html#targetText=Prefix-,Cats,-Suffix");
request.Complete(R"HTML( request.Complete(R"HTML(
<!DOCTYPE html> <!DOCTYPE html>
<style> <style>
...@@ -1010,6 +1010,31 @@ TEST_F(TextFragmentAnchorTest, DisabledInSamePageNavigation) { ...@@ -1010,6 +1010,31 @@ TEST_F(TextFragmentAnchorTest, DisabledInSamePageNavigation) {
EXPECT_EQ(ScrollOffset(), LayoutViewport()->GetScrollOffset()); EXPECT_EQ(ScrollOffset(), LayoutViewport()->GetScrollOffset());
} }
// Make sure matching is case sensitive.
TEST_F(TextFragmentAnchorTest, CaseSensitive) {
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">test</p>
)HTML");
Compositor().BeginFrame();
RunAsyncMatchingTasks();
EXPECT_EQ(ScrollOffset(), LayoutViewport()->GetScrollOffset());
EXPECT_TRUE(GetDocument().Markers().Markers().IsEmpty());
}
} // namespace } // namespace
} // namespace blink } // namespace blink
third_party/blink/renderer/core/page/scrolling/text_fragment_finder.cc
View file @ f55c0034
...@@ -24,9 +24,9 @@ const char kNoContext[] = ""; ...@@ -24,9 +24,9 @@ const char kNoContext[] = "";
EphemeralRangeInFlatTree FindMatchInRange(String search_text, EphemeralRangeInFlatTree FindMatchInRange(String search_text,
PositionInFlatTree search_start, PositionInFlatTree search_start,
PositionInFlatTree search_end) { PositionInFlatTree search_end) {
const FindOptions find_options = kCaseInsensitive;
const EphemeralRangeInFlatTree search_range(search_start, search_end); const EphemeralRangeInFlatTree search_range(search_start, search_end);
return FindBuffer::FindMatchInRange(search_range, search_text, find_options); return FindBuffer::FindMatchInRange(search_range, search_text,
/*find_options=*/0);
} }
PositionInFlatTree NextTextPosition(PositionInFlatTree position, PositionInFlatTree NextTextPosition(PositionInFlatTree position,
...@@ -56,10 +56,9 @@ EphemeralRangeInFlatTree FindImmediateMatch(String search_text, ...@@ -56,10 +56,9 @@ EphemeralRangeInFlatTree FindImmediateMatch(String search_text,
return EphemeralRangeInFlatTree(); return EphemeralRangeInFlatTree();
FindBuffer buffer(EphemeralRangeInFlatTree(search_start, search_end)); FindBuffer buffer(EphemeralRangeInFlatTree(search_start, search_end));
const FindOptions find_options = kCaseInsensitive;
std::unique_ptr<FindBuffer::Results> match_results = std::unique_ptr<FindBuffer::Results> match_results =
buffer.FindMatches(search_text, find_options); buffer.FindMatches(search_text, /*find_options=*/0);
if (!match_results->IsEmpty() && match_results->front().start == 0u) { if (!match_results->IsEmpty() && match_results->front().start == 0u) {
FindBuffer::BufferMatchResult match = match_results->front(); FindBuffer::BufferMatchResult match = match_results->front();
......
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