Skip to content
Permalink
Browse files
Fancy Quotes and Apostrophes are not matched to Scroll To Text Fragme…
…nt fragments.

https://bugs.webkit.org/show_bug.cgi?id=244918

Reviewed by Wenson Hsieh.

Leverage existing quote folding code to match non-ascii quote marks for more accurate
fragment matching.

* LayoutTests/http/tests/scroll-to-text-fragment/start-text-quote-expected.html: Added.
* LayoutTests/http/tests/scroll-to-text-fragment/start-text-quote.html: Added.
* Source/WebCore/dom/FragmentDirectiveRangeFinder.cpp:
(WebCore::FragmentDirectiveRangeFinder::findRangeFromNodeList):

Canonical link: https://commits.webkit.org/254275@main
  • Loading branch information
megangardner committed Sep 8, 2022
1 parent 768cdfe commit c79c0dc354f2430992eebd89c515da6bc39406ea
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 0 deletions.
@@ -0,0 +1,12 @@
<!DOCTYPE html>
<html lang="en">
<meta charset="utf-8" />
<title>Scroll to text fragment - highlight simple start reference</title>
<style>
span {
background-color: rgb(255, 238, 190);
}
</style>

<p>The test passes if the following word has a yellow background.</p>
<div>Select <span>“isn’t”</span> to pass.</div>
@@ -0,0 +1,13 @@
<!DOCTYPE html><!-- webkit-test-runner [ ScrollToTextFragmentIndicatorEnabled=false ] -->
<meta charset="utf-8" />
<title>Scroll to text fragment - highlight simple start text</title>
<link rel="help" href="https://wicg.github.io/scroll-to-text-fragment/">
<meta name="assert" content="This test checks that a fragment directive with quotes and apostrophes is correctly highlighted.">

<p>The test passes if the following word has a yellow background.</p>
<div>Select <span>“isn’t”</span> to pass.</div>

<script>
location.href = "#:~:text=\"isn't\"";
</script>
</html>
@@ -145,6 +145,8 @@ static std::optional<SimpleRange> findRangeFromNodeList(const String& query, con
// FIXME: try to use SearchBuffer in TextIterator.h instead.

This comment has been minimized.

Copy link
@darinadler

darinadler Sep 8, 2022

Member

Using the search code in TextIterator.h would be an important enhancement. That code goes out of its way to use the search capabilities of ICU’s usearch as well as a few other things we need to do in WebKit because ICU doesn’t handle them, and the reason we created that code was to avoid having separate search logic elsewhere in WebKit that might leave our some subtle requirements for correct search.

One downside of moving to that search is that its definition of visible text may not match what the specification calls for. That part of the specification seems to currently be in flux.

Another downside of moving to that search is that what it calls for is not exactly the one in TextIterator.h; we’d have to parameterize it to turn off anything that is not desired, including the different searching based on the language of the user. That kind of context sensitivity isn’t a plus if we want to create durable links that work the same way for multiple users.

Reading the current specification, which is still in flux about what text should be searched, it’s clear that the combination of quote mark folding and ignoring ASCII case is not a complete implementation, since the specification calls for "using a base character comparison, or the primary level http://www.unicode.org/reports/tr10/#Multi_Level_Comparison, as defined in UTS10 https://wicg.github.io/scroll-to-text-fragment/#biblio-uts10". I believe it would be easier to do as I suggest above and configure TextIterator.h to support that set of options than to write correct new code here.

searchBuffer = searchBufferBuilder.toString();

searchBuffer = foldQuoteMarks(searchBuffer);

unsigned searchStart = 0;

if (nodes[0].ptr() == &searchRange.startContainer())

0 comments on commit c79c0dc

Please sign in to comment.