Skip to content

GitLab

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

[text-fragment] Fix prefix matching edge case 

The current algorithm first finds a |prefix|, then checks if the
immediate following text is the |search_text|. If it is it'll advance
|search_start| to the end of the |search_text| so it can confirm it's
followed by the |suffix| text. However, this means that if the |suffix|
text doesn't match, the next iteration of the algorithm will start
searching for the |prefix| at the end of the |search_text|. This is a
problem if the |search_text| == |prefix| because we've now skipped the
|search_text| part of the range for prefix matching.

E.g. With prefix=foo search_text=foo suffix=bar we'll fail to match:

foo foo foo bar

Because the first iteration of the loop will use the first "foo" as
the prefix, and the next iteration will use the third "foo" as the
prefix; we skip the second.

Bug: 1054243
Change-Id: I62b8a32f89c987e5bed8ea439ae1b7d53d12436d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2065979Reviewed-by: default avatarNick Burris <nburris@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743108}
parent 63ad1ff7
Hide whitespace changes
Inline Side-by-side
Showing with 15 additions and 3 deletions
third_party/blink/renderer/core/page/scrolling/text_fragment_finder.cc
View file @ d6226d97
...@@ -148,13 +148,15 @@ EphemeralRangeInFlatTree FindMatchInRangeWithContext( ...@@ -148,13 +148,15 @@ EphemeralRangeInFlatTree FindMatchInRangeWithContext(
// No search_text match in remaining range // No search_text match in remaining range
if (potential_match.IsNull()) if (potential_match.IsNull())
return EphemeralRangeInFlatTree(); return EphemeralRangeInFlatTree();
search_start = potential_match.EndPosition();
} }
PositionInFlatTree suffix_start = potential_match.EndPosition();
DCHECK(potential_match.IsNotNull()); DCHECK(potential_match.IsNotNull());
search_start = potential_match.EndPosition();
if (!suffix.IsEmpty()) { if (!suffix.IsEmpty()) {
EphemeralRangeInFlatTree suffix_match = EphemeralRangeInFlatTree suffix_match =
FindImmediateMatch(suffix, search_start, search_end); FindImmediateMatch(suffix, suffix_start, search_end);
// No suffix match after current potential_match // No suffix match after current potential_match
if (suffix_match.IsNull()) if (suffix_match.IsNull())
......
third_party/blink/web_tests/external/wpt/scroll-to-text-fragment/scroll-to-text-fragment-target.html
View file @ d6226d97
...@@ -74,7 +74,11 @@ window.onload = function() { ...@@ -74,7 +74,11 @@ window.onload = function() {
</style> </style>
<body> <body>
<div id="element" class="scroll-section">Element</div> <div id="element" class="scroll-section">Element</div>
<p id="text" class="scroll-section">This is a test page !$'()*+./:;=?@_~ &,- &#x30cd;&#x30b3;</p> <p id="text" class="scroll-section">
This is a test page !$'()*+./:;=?@_~ &,- &#x30cd;&#x30b3;
<br>
foo foo foo bar bar bar
</p>
<p id="more-text" class="scroll-section">More test page text</p> <p id="more-text" class="scroll-section">More test page text</p>
<div class="scroll-section"> <div class="scroll-section">
<div> <div>
......
third_party/blink/web_tests/external/wpt/scroll-to-text-fragment/scroll-to-text-fragment.html
View file @ d6226d97
...@@ -61,6 +61,12 @@ let test_cases = [ ...@@ -61,6 +61,12 @@ let test_cases = [
expect_position: 'text', expect_position: 'text',
description: 'Exact text with prefix and suffix should match text' description: 'Exact text with prefix and suffix should match text'
}, },
// Test tricky edge case where prefix and query are equal
{
fragment: '#:~:text=foo-,foo,-bar',
expect_position: 'text',
description: 'Exact text with prefix and suffix and query equals prefix.'
},
// Test text range matching, with all combinations of context terms // Test text range matching, with all combinations of context terms
{ {
fragment: '#:~:text=this,page', fragment: '#:~:text=this,page',
......
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