Modified: trunk/Source/WebCore/ChangeLog (264119 => 264120)
--- trunk/Source/WebCore/ChangeLog 2020-07-08 18:15:07 UTC (rev 264119)
+++ trunk/Source/WebCore/ChangeLog 2020-07-08 18:30:44 UTC (rev 264120)
@@ -1,3 +1,33 @@
+2020-07-08 Sihui Liu <[email protected]>
+
+ Text manipulation should ignore white spaces between nodes
+ https://bugs.webkit.org/show_bug.cgi?id=213907
+
+ Reviewed by Wenson Hsieh.
+
+ Text returned by TextIterator contains white spaces (including tabs and line breaks) that do not belong to
+ content of nodes. Those spaces are emitted based on style of nodes. For example, line breaks can be emitted
+ before and after block-level element. These spaces should not be extracted as part of the content, because
+ they could change based on the style of nodes or range of TextIterator, and manipulation fails if content is
+ changed. We want to make sure TextManipulationController monitors the real content of nodes.
+
+ r262778 tried solving this issue by excluding text with empty node from TextIterator's result. That worked with
+ line break, but not space and tab. See radar and new test. To solve this, now we exclude text with zero-length
+ range.
+
+ This patch does not change the behavior of line breaks, which is covered by:
+ TextManipulation.StartTextManipulationExtractsVisibleLineBreaksInTextAsExcludedTokens
+ TextManipulation.CompleteTextManipulationCanMergeContentAndPreserveLineBreaks
+ TextManipulation.CompleteTextManipulationReplaceTwoSimpleParagraphs
+
+ New test: TextManipulation.CompleteTextManipulationIgnoreWhiteSpacesBetweenParagraphs
+ Modified existing tests: TextManipulation.StartTextManipulationExtractsValuesByNode
+
+ * editing/TextManipulationController.cpp:
+ (WebCore::ParagraphContentIterator::ParagraphContentIterator):
+ (WebCore::ParagraphContentIterator::shouldAdvanceIteratorPastCurrentNode const):
+ (WebCore::ParagraphContentIterator::advanceIteratorNodeAndUpdateText):
+
2020-07-08 Darin Adler <[email protected]>
Remove use of live ranges from AXObject.h
Modified: trunk/Source/WebCore/editing/TextManipulationController.cpp (264119 => 264120)
--- trunk/Source/WebCore/editing/TextManipulationController.cpp 2020-07-08 18:15:07 UTC (rev 264119)
+++ trunk/Source/WebCore/editing/TextManipulationController.cpp 2020-07-08 18:30:44 UTC (rev 264120)
@@ -147,7 +147,6 @@
public:
ParagraphContentIterator(const Position& start, const Position& end)
: m_iterator({ *makeBoundaryPoint(start), *makeBoundaryPoint(end) }, TextIteratorIgnoresStyleVisibility)
- , m_iteratorNode(m_iterator.atEnd() ? nullptr : createLiveRange(m_iterator.range())->firstNode())
, m_node(start.firstNode())
, m_pastEndNode(end.firstNode())
{
@@ -188,8 +187,15 @@
bool atEnd() const { return !m_text && m_iterator.atEnd() && m_node == m_pastEndNode; }
private:
- bool shouldAdvanceIteratorPastCurrentNode() const { return !m_iterator.atEnd() && m_iteratorNode == m_node; }
+ bool shouldAdvanceIteratorPastCurrentNode() const
+ {
+ if (m_iterator.atEnd())
+ return false;
+ auto* iteratorNode = m_iterator.node();
+ return !iteratorNode || iteratorNode == m_node;
+ }
+
void advanceNode()
{
if (m_node == m_pastEndNode)
@@ -215,21 +221,16 @@
StringBuilder stringBuilder;
Vector<String> text;
while (shouldAdvanceIteratorPastCurrentNode()) {
- if (!m_iterator.node()) {
- auto iteratorText = m_iterator.text();
- bool containsDelimiter = false;
- for (unsigned index = 0; index < iteratorText.length() && !containsDelimiter; ++index)
- containsDelimiter = isTokenDelimiter(iteratorText[index]);
-
- if (containsDelimiter) {
+ auto iteratorText = m_iterator.text();
+ if (m_iterator.range().collapsed()) {
+ if (iteratorText == "\n") {
appendToText(text, stringBuilder);
text.append({ });
}
} else
- stringBuilder.append(m_iterator.text());
+ stringBuilder.append(iteratorText);
m_iterator.advance();
- m_iteratorNode = m_iterator.atEnd() ? nullptr : createLiveRange(m_iterator.range())->firstNode();
}
appendToText(text, stringBuilder);
m_text = text;
@@ -236,7 +237,6 @@
}
TextIterator m_iterator;
- RefPtr<Node> m_iteratorNode;
RefPtr<Node> m_node;
RefPtr<Node> m_pastEndNode;
Optional<Vector<String>> m_text;
Modified: trunk/Tools/ChangeLog (264119 => 264120)
--- trunk/Tools/ChangeLog 2020-07-08 18:15:07 UTC (rev 264119)
+++ trunk/Tools/ChangeLog 2020-07-08 18:30:44 UTC (rev 264120)
@@ -1,3 +1,13 @@
+2020-07-08 Sihui Liu <[email protected]>
+
+ Text manipulation should ignore white spaces between nodes
+ https://bugs.webkit.org/show_bug.cgi?id=213907
+
+ Reviewed by Wenson Hsieh.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
+ (TestWebKitAPI::TEST):
+
2020-07-08 Andy Estes <[email protected]>
REGRESSION (r207558): [Mac] Context clicks on links sometimes result in navigations
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm (264119 => 264120)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2020-07-08 18:15:07 UTC (rev 264119)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2020-07-08 18:30:44 UTC (rev 264120)
@@ -983,6 +983,7 @@
"<body>"
"<span>one</span><span style='white-space:pre;'>two three</span><span>four</span>"
"<span style='white-space:pre;'> five</span>"
+ "<span> six</span> <span>seven</span>"
"</body>"];
done = false;
@@ -994,14 +995,17 @@
auto items = [delegate items];
EXPECT_EQ(items.count, 2UL);
EXPECT_EQ(items[0].tokens.count, 5UL);
- EXPECT_STREQ("one", items[0].tokens[0].content.UTF8String);
- EXPECT_STREQ("two", items[0].tokens[1].content.UTF8String);
- EXPECT_STREQ(" \n ", items[0].tokens[2].content.UTF8String);
- EXPECT_STREQ("three", items[0].tokens[3].content.UTF8String);
- EXPECT_STREQ("four", items[0].tokens[4].content.UTF8String);
- EXPECT_EQ(items[1].tokens.count, 2UL);
- EXPECT_STREQ("\n", items[1].tokens[0].content.UTF8String);
- EXPECT_STREQ("five", items[1].tokens[1].content.UTF8String);
+ EXPECT_WK_STREQ("one", items[0].tokens[0].content);
+ EXPECT_WK_STREQ("two", items[0].tokens[1].content);
+ EXPECT_WK_STREQ(" \n ", items[0].tokens[2].content);
+ EXPECT_WK_STREQ("three", items[0].tokens[3].content);
+ EXPECT_WK_STREQ("four", items[0].tokens[4].content);
+ EXPECT_EQ(items[1].tokens.count, 5UL);
+ EXPECT_WK_STREQ("\n", items[1].tokens[0].content);
+ EXPECT_WK_STREQ("five", items[1].tokens[1].content);
+ EXPECT_WK_STREQ(" six", items[1].tokens[2].content);
+ EXPECT_WK_STREQ(" ", items[1].tokens[3].content);
+ EXPECT_WK_STREQ("seven", items[1].tokens[4].content);
}
TEST(TextManipulation, StartTextManipulationExcludesTextRenderedAsIcons)
@@ -2444,6 +2448,48 @@
EXPECT_WK_STREQ("<span style=\"white-space:pre;\">ONE \n TWO \n</span>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
}
+TEST(TextManipulation, CompleteTextManipulationIgnoreWhiteSpacesBetweenParagraphs)
+{
+ auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
+ [webView _setTextManipulationDelegate:delegate.get()];
+ [webView synchronouslyLoadHTMLString:@""
+ "<style>"
+ ".inline-block { display: inline-block; }"
+ ".list-item { display: list-item; }"
+ ".hide-absolute { display: none; position: absolute; }"
+ ".float-relative { float: left; position: relative; }"
+ "</style>"
+ "<ul class='float-relative'>"
+ " <li class='list-item float-relative'><a class='inline-block'>hello</a> <div class='hide-absolute'><a class='inline-block float-relative'>hide</a></div></li>"
+ " <li class='list-item float-relative'><a class='inline-block'>world</a></li>"
+ "</ul>"];
+ auto configuration = adoptNS([[_WKTextManipulationConfiguration alloc] init]);
+
+ done = false;
+ [webView _startTextManipulationsWithConfiguration:configuration.get() completion:^{
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+
+ auto *items = [delegate items];
+ EXPECT_EQ(items.count, 2UL);
+ EXPECT_EQ(items[0].tokens.count, 1UL);
+ EXPECT_WK_STREQ("hello", items[0].tokens[0].content);
+ EXPECT_EQ(items[1].tokens.count, 1UL);
+ EXPECT_WK_STREQ("world", items[1].tokens[0].content);
+
+ done = false;
+ [webView _completeTextManipulationForItems:@[
+ createItem(items[0].identifier, {{ items[0].tokens[0].identifier, @"Hello" }}).get(),
+ createItem(items[1].identifier, {{ items[1].tokens[0].identifier, @"World" }}).get()
+ ] completion:^(NSArray<NSError *> *errors) {
+ EXPECT_EQ(errors, nil);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+}
+
TEST(TextManipulation, CompleteTextManipulationShouldPreserveNodesAfterParagraphRange)
{
auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
