Commit 7ebaeaf2 authored by Gayane Petrosyan's avatar Gayane Petrosyan Committed by Chromium LUCI CQ

[Sh-Blink] Fix for non text node positions

Fix by replacing the non text node position by its child text node,
if possible. Otherwise, skip the word-completion logic, which assumes,
the position is in text node.

Bug: 1165317
Change-Id: Iafb82eb1f1f7e5b6644cc07034166e6f0344d184
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2622987
Commit-Queue: Gayane Petrosyan <gayane@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843765}
parent c6c3682e
......@@ -28,9 +28,10 @@ namespace {
bool IsFirstVisiblePosition(Node* node, unsigned pos_offset) {
auto range_start = Position::FirstPositionInNode(*node);
auto range_end = Position(node, pos_offset);
return pos_offset == 0 || PlainText(EphemeralRange(range_start, range_end))
.StripWhiteSpace()
.IsEmpty();
return node->getNodeType() == Node::kElementNode || pos_offset == 0 ||
PlainText(EphemeralRange(range_start, range_end))
.StripWhiteSpace()
.IsEmpty();
}
// Returns true if text from |pos_offset| until end of |node| can be considered
......@@ -38,7 +39,8 @@ bool IsFirstVisiblePosition(Node* node, unsigned pos_offset) {
bool IsLastVisiblePosition(Node* node, unsigned pos_offset) {
auto range_start = Position(node, pos_offset);
auto range_end = Position::LastPositionInNode(*node);
return pos_offset == node->textContent().length() ||
return node->getNodeType() == Node::kElementNode ||
pos_offset == node->textContent().length() ||
PlainText(EphemeralRange(range_start, range_end))
.StripWhiteSpace()
.IsEmpty();
......@@ -152,6 +154,19 @@ String GetWordsFromStart(String text, int word_num) {
return text.Substring(0, pos).StripWhiteSpace();
}
// For Element-based Position returns the node that its pointing to, otherwise
// returns the container node.
Node* ResolvePositionToNode(const PositionInFlatTree& position) {
Node* node = position.ComputeContainerNode();
int offset = position.ComputeOffsetInContainerNode();
if (node->getNodeType() == Node::kElementNode && node->hasChildren() &&
node->childNodes()->item(offset)) {
return node->childNodes()->item(offset);
}
return node;
}
} // namespace
constexpr int kExactTextMaxChars = 300;
......@@ -195,13 +210,23 @@ void TextFragmentSelectorGenerator::AdjustSelection() {
ephemeral_range.StartPosition().ComputeContainerNode();
Node* end_container = ephemeral_range.EndPosition().ComputeContainerNode();
Node* corrected_start =
ResolvePositionToNode(ephemeral_range.StartPosition());
int corrected_start_offset =
(corrected_start->isSameNode(start_container))
? ephemeral_range.StartPosition().ComputeOffsetInContainerNode()
: 0;
Node* corrected_end = ResolvePositionToNode(ephemeral_range.EndPosition());
int corrected_end_offset =
(corrected_end->isSameNode(end_container))
? ephemeral_range.EndPosition().ComputeOffsetInContainerNode()
: 0;
// If start node has no text or given start position point to the last visible
// text in its containiner node, use the following visible node for selection
// start. This has to happen before generation, so that selection is correctly
// classified as same block or not.
Node* corrected_start = start_container;
int corrected_start_offset =
ephemeral_range.StartPosition().ComputeOffsetInContainerNode();
if (IsLastVisiblePosition(corrected_start, corrected_start_offset)) {
corrected_start = FirstNonEmptyVisibleTextNode(
FlatTreeTraversal::NextSkippingChildren(*corrected_start));
......@@ -220,9 +245,6 @@ void TextFragmentSelectorGenerator::AdjustSelection() {
// text in its containiner node, use the previous visible node for selection
// end. This has to happen before generation, so that selection is correctly
// classified as same block or not.
Node* corrected_end = end_container;
int corrected_end_offset =
ephemeral_range.EndPosition().ComputeOffsetInContainerNode();
if (IsFirstVisiblePosition(corrected_end, corrected_end_offset)) {
// Here, |Previous()| already skips the children of the given node,
// because we're doing pre-order traversal.
......
......@@ -947,6 +947,72 @@ TEST_F(TextFragmentSelectorGeneratorTest, RangeSelector_SameNode_Interrupted) {
VerifySelector(start, end, "First,paragraph");
}
// Checks the case when selection end position is a non text node.
TEST_F(TextFragmentSelectorGeneratorTest, SelectionEndsWithNonText) {
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Complete(R"HTML(
<!DOCTYPE html>
<div id='div'>
<p id='start'>First paragraph</p>
<p id='second'>Second paragraph</p>
</div>
)HTML");
GetDocument().UpdateStyleAndLayoutTree();
Node* first_paragraph = GetDocument().getElementById("start")->firstChild();
Node* div = GetDocument().getElementById("div");
const auto& start = Position(first_paragraph, 0);
const auto& end = Position(div, 2);
ASSERT_EQ("First paragraph\n\n", PlainText(EphemeralRange(start, end)));
VerifySelector(start, end, "First%20paragraph,-Second");
}
// Checks the case when selection end position is a non text node which doesn't
// have text child node.
TEST_F(TextFragmentSelectorGeneratorTest,
SelectionEndsWithNonTextWithNoTextChild) {
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Complete(R"HTML(
<!DOCTYPE html>
<div id='div'><p id='start'>First paragraph</p><p id='second'>Second paragraph</p><img id="img">
</div>
)HTML");
GetDocument().UpdateStyleAndLayoutTree();
Node* first_paragraph = GetDocument().getElementById("start")->firstChild();
Node* div = GetDocument().getElementById("div");
const auto& start = Position(first_paragraph, 0);
const auto& end =
Position(div, 3); // Points to the 3rd child of the div, which is <img>
ASSERT_EQ("First paragraph\n\nSecond paragraph\n\n",
PlainText(EphemeralRange(start, end)));
VerifySelector(start, end, "First%20paragraph,Second%20paragraph");
}
// Checks the case when selection end position is a non text node which doesn't
// have text child node.
TEST_F(TextFragmentSelectorGeneratorTest, SelectionEndsWithImageDiv) {
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Complete(R"HTML(
<!DOCTYPE html>
<div id='div'><p id='start'>First paragraph</p><p id='second'>Second paragraph</p><div id='div_img'><img id="img"></div>
</div>
)HTML");
GetDocument().UpdateStyleAndLayoutTree();
Node* first_paragraph = GetDocument().getElementById("start")->firstChild();
Node* div = GetDocument().getElementById("div");
const auto& start = Position(first_paragraph, 0);
const auto& end =
Position(div, 3); // Points to the 3rd child of the div, which is div_img
ASSERT_EQ("First paragraph\n\nSecond paragraph\n\n",
PlainText(EphemeralRange(start, end)));
VerifySelector(start, end, "First%20paragraph,Second%20paragraph");
}
// Basic test case for |GetNextTextBlock|.
TEST_F(TextFragmentSelectorGeneratorTest, GetPreviousTextBlock) {
SimRequest request("https://example.com/test.html", "text/html");
......
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