Skip to content

GitLab

T
tangled
Commit 9466cd3a authored by Gayane Petrosyan's avatar Gayane Petrosyan Committed by Commit Bot
Browse files
Options

[SH-Blink] Improve when a node is considered empty. 

In some cases PlainText might return a single space " ". The case found
is when a selection starts with an element that is nested within
inline-block element. This combination creates extra spaces that is
selectable and is not fully collapsed by PlainText.

Bug: 1151474
Change-Id: Id855e9847efc5f70cedb3c58d72b33191c4771b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2558914
Commit-Queue: Gayane Petrosyan <gayane@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#830772}
parent d217de9f
Hide whitespace changes
Inline Side-by-side
Showing with 40 additions and 6 deletions
third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.cc
View file @ 9466cd3a
...@@ -20,11 +20,6 @@ namespace blink { ...@@ -20,11 +20,6 @@ namespace blink {
namespace { namespace {
// Returns text content of the node, skipping invisible children and comments.
String GetText(Node* node) {
return PlainText(EphemeralRange::RangeOfContents(*node));
}
// Returns true if text from beginning of |node| until |pos_offset| can be // Returns true if text from beginning of |node| until |pos_offset| can be
// considered empty. Otherwise, return false. // considered empty. Otherwise, return false.
bool IsFirstVisiblePosition(Node* node, unsigned pos_offset) { bool IsFirstVisiblePosition(Node* node, unsigned pos_offset) {
...@@ -76,7 +71,9 @@ Node* NextNonEmptyVisibleTextNode(Node* start_node) { ...@@ -76,7 +71,9 @@ Node* NextNonEmptyVisibleTextNode(Node* start_node) {
// Move forward/backward until non empty visible text node is found. // Move forward/backward until non empty visible text node is found.
for (Node* node = start_node; node; node = Direction::Next(*node)) { for (Node* node = start_node; node; node = Direction::Next(*node)) {
Node* next_node = Direction::GetVisibleTextNode(*node); Node* next_node = Direction::GetVisibleTextNode(*node);
if (!next_node || !GetText(next_node).IsEmpty()) if (!next_node || !PlainText(EphemeralRange::RangeOfContents(*next_node))
.StripWhiteSpace()
.IsEmpty())
return next_node; return next_node;
node = next_node; node = next_node;
} }
......
third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator_test.cc
View file @ 9466cd3a
...@@ -717,6 +717,43 @@ TEST_F(TextFragmentSelectorGeneratorTest, StartsWithBlockWithImage) { ...@@ -717,6 +717,43 @@ TEST_F(TextFragmentSelectorGeneratorTest, StartsWithBlockWithImage) {
GenerateAndVerifySelector(start, end, "page-,First,-paragraph"); GenerateAndVerifySelector(start, end, "page-,First,-paragraph");
} }
// Check the case when selections starts with a node nested in "inline-block"
// node. crbug.com/1151474
TEST_F(TextFragmentSelectorGeneratorTest, StartsWithInlineBlockChild) {
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
li {
display: inline-block;
}
</style>
<div>Test page</div>
<ul>
<li>
<a id="link1"/>
</li>
<li>
<a id="link2"/>
</li>
<li>
<a id="link3"/>
</li>
</ul>
<p id='first'>First paragraph text that is longer than 20 chars</p>
)HTML");
GetDocument().View()->UpdateAllLifecyclePhasesForTest();
Node* img = GetDocument().getElementById("link1");
Node* first_paragraph = GetDocument().getElementById("first")->firstChild();
const auto& start = Position(img, PositionAnchorType::kAfterChildren);
const auto& end = Position(first_paragraph, 5);
ASSERT_EQ(" \nFirst", PlainText(EphemeralRange(start, end)));
GenerateAndVerifySelector(start, end, "page-,First,-paragraph");
}
// Check the case when selections ends with an non text node. // Check the case when selections ends with an non text node.
TEST_F(TextFragmentSelectorGeneratorTest, EndswithImage) { TEST_F(TextFragmentSelectorGeneratorTest, EndswithImage) {
SimRequest request("https://example.com/test.html", "text/html"); 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