Skip to content

GitLab

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

Make TextFragmentAnchor case insensitive 

Revert TextFragmentAnchor matching behavior back to case insensitive.
Case sensitive reduced ambiguity, but also reduced match rate on CSS
transformed text and &nbsp (see bug 982031). Ambiguity can, in most
cases, be reduced by providing more context terms, so case insensitive
is probably best.

Bug: 982031, 982035
Change-Id: I633c674affb16b571294e5d81b6ae3b24ec9a731
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729550Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Commit-Queue: Nick Burris <nburris@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682859}
parent 8fea5a10
Hide whitespace changes
Inline Side-by-side
Showing with 75 additions and 6 deletions
third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_test.cc
View file @ 73bd0c83
......@@ -1010,8 +1010,8 @@ TEST_F(TextFragmentAnchorTest, DisabledInSamePageNavigation) {
EXPECT_EQ(ScrollOffset(), LayoutViewport()->GetScrollOffset());
}
// Make sure matching is case sensitive.
TEST_F(TextFragmentAnchorTest, CaseSensitive) {
// Ensure matching is case insensitive.
TEST_F(TextFragmentAnchorTest, CaseInsensitive) {
SimRequest request("https://example.com/test.html#targetText=Test",
"text/html");
LoadURL("https://example.com/test.html#targetText=Test");
......@@ -1031,8 +1031,13 @@ TEST_F(TextFragmentAnchorTest, CaseSensitive) {
Compositor().BeginFrame();
RunAsyncMatchingTasks();
EXPECT_EQ(ScrollOffset(), LayoutViewport()->GetScrollOffset());
EXPECT_TRUE(GetDocument().Markers().Markers().IsEmpty());
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();
EXPECT_EQ(1u, GetDocument().Markers().Markers().size());
}
// Test that the fragment anchor stays centered in view throughout loading.
......@@ -1212,6 +1217,69 @@ TEST_F(TextFragmentAnchorTest, IdFragmentWithDoubleHash) {
<< LayoutViewport()->GetScrollOffset().ToString();
}
// Test matching a space to &nbsp character.
TEST_F(TextFragmentAnchorTest, SpaceMatchesNbsp) {
SimRequest request("https://example.com/test.html#targetText=test%20page",
"text/html");
LoadURL("https://example.com/test.html#targetText=test%20page");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
body {
height: 1200px;
}
p {
position: absolute;
top: 1000px;
}
</style>
<p id="text">This is a test&nbsp;page</p>
)HTML");
Compositor().BeginFrame();
RunAsyncMatchingTasks();
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();
EXPECT_EQ(1u, GetDocument().Markers().Markers().size());
}
// Test matching text with a CSS text transform.
TEST_F(TextFragmentAnchorTest, CSSTextTransform) {
SimRequest request("https://example.com/test.html#targetText=test%20page",
"text/html");
LoadURL("https://example.com/test.html#targetText=test%20page");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
body {
height: 1200px;
}
p {
position: absolute;
top: 1000px;
text-transform: uppercase;
}
</style>
<p id="text">This is a test page</p>
)HTML");
Compositor().BeginFrame();
RunAsyncMatchingTasks();
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();
EXPECT_EQ(1u, GetDocument().Markers().Markers().size());
}
} // namespace
} // namespace blink
third_party/blink/renderer/core/page/scrolling/text_fragment_finder.cc
View file @ 73bd0c83
......@@ -11,6 +11,7 @@
#include "third_party/blink/renderer/core/dom/range.h"
#include "third_party/blink/renderer/core/editing/ephemeral_range.h"
#include "third_party/blink/renderer/core/editing/finder/find_buffer.h"
#include "third_party/blink/renderer/core/editing/finder/find_options.h"
#include "third_party/blink/renderer/core/editing/iterators/character_iterator.h"
#include "third_party/blink/renderer/core/editing/position.h"
#include "third_party/blink/renderer/core/page/scrolling/text_fragment_selector.h"
......@@ -26,7 +27,7 @@ EphemeralRangeInFlatTree FindMatchInRange(String search_text,
PositionInFlatTree search_end) {
const EphemeralRangeInFlatTree search_range(search_start, search_end);
return FindBuffer::FindMatchInRange(search_range, search_text,
/*find_options=*/0);
kCaseInsensitive);
}
PositionInFlatTree NextTextPosition(PositionInFlatTree position,
......@@ -58,7 +59,7 @@ EphemeralRangeInFlatTree FindImmediateMatch(String search_text,
FindBuffer buffer(EphemeralRangeInFlatTree(search_start, search_end));
std::unique_ptr<FindBuffer::Results> match_results =
buffer.FindMatches(search_text, /*find_options=*/0);
buffer.FindMatches(search_text, kCaseInsensitive);
if (!match_results->IsEmpty() && match_results->front().start == 0u) {
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