Commit e7ab70be authored by Nick Burris's avatar Nick Burris Committed by Commit Bot

Make FindBuffer ignore comment nodes

Ensure comment nodes don't create an editing block boundary. This allows
find-in-page as well as text fragments to match text that spans inline
nodes that are separated by inline HTML comments, e.g. we expect to be
able to match "abcdef" in the following block.

<span>abc</span><!--comment--><span>def</span>

Bug: 982055
Change-Id: Ie1cd2d9d1837e052e29abc3f4ebbf7e609401367
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2204631Reviewed-by: default avatarRakina Zata Amni <rakina@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Commit-Queue: Nick Burris <nburris@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770641}
parent a7b703ec
......@@ -28,6 +28,10 @@ namespace {
// Returns true if the search should ignore the given |node|'s contents. In
// other words, we don't need to recurse into the node's children.
bool ShouldIgnoreContents(const Node& node) {
if (node.getNodeType() == Node::kCommentNode) {
return true;
}
const auto* element = DynamicTo<HTMLElement>(node);
if (!element)
return false;
......@@ -244,8 +248,10 @@ void FindBuffer::CollectTextUntilBlockBoundary(
}
// Move the node so we wouldn't encounter this node or its descendants
// later.
if (!IsA<HTMLWBRElement>(To<HTMLElement>(*node)))
if (IsA<HTMLElement>(*node) &&
!IsA<HTMLWBRElement>(To<HTMLElement>(*node))) {
buffer_.push_back(kMaxCodepoint);
}
node = FlatTreeTraversal::NextSkippingChildren(*node);
continue;
}
......
......@@ -717,4 +717,20 @@ TEST_F(TextFinderTest, BeforeMatchEventRemoveElement) {
// TODO(jarhar): Write more tests here once we decide on a behavior here:
// https://github.com/WICG/display-locking/issues/150
TEST_F(TextFinderTest, FindTextAcrossCommentNode) {
GetDocument().body()->setInnerHTML(
"<span>abc</span><!--comment--><span>def</span>");
GetDocument().UpdateStyleAndLayout(DocumentUpdateReason::kTest);
int identifier = 0;
WebString search_text(String("abcdef"));
auto find_options = mojom::blink::FindOptions::New();
find_options->run_synchronously_for_testing = true;
bool wrap_within_frame = true;
EXPECT_TRUE(GetTextFinder().Find(identifier, search_text, *find_options,
wrap_within_frame));
EXPECT_TRUE(GetTextFinder().ActiveMatch());
}
} // namespace blink
......@@ -1861,6 +1861,37 @@ TEST_F(TextFragmentAnchorTest, PageVisibility) {
EXPECT_EQ(p, *GetDocument().CssTarget());
}
// Test that a text directive can match across comment nodes
TEST_F(TextFragmentAnchorTest, MatchAcrossCommentNode) {
SimRequest request("https://example.com/test.html#:~:text=abcdef",
"text/html");
LoadURL("https://example.com/test.html#:~:text=abcdef");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
body {
height: 1200px;
}
div {
position: absolute;
top: 1000px;
}
</style>
<div id="text"><span>abc</span><!--comment--><span>def</span></div>
)HTML");
RunAsyncMatchingTasks();
// Render two frames to handle the async step added by the beforematch event.
Compositor().BeginFrame();
Compositor().BeginFrame();
Element& div = *GetDocument().getElementById("text");
EXPECT_EQ(div, *GetDocument().CssTarget());
EXPECT_TRUE(ViewportRect().Contains(BoundingRectInFrame(div)));
EXPECT_EQ(2u, GetDocument().Markers().Markers().size());
}
} // namespace
} // 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