Diff
Modified: branches/safari-610.1.7-branch/Source/WebCore/ChangeLog (258375 => 258376)
--- branches/safari-610.1.7-branch/Source/WebCore/ChangeLog 2020-03-13 00:47:11 UTC (rev 258375)
+++ branches/safari-610.1.7-branch/Source/WebCore/ChangeLog 2020-03-13 00:51:38 UTC (rev 258376)
@@ -1,5 +1,9 @@
2020-03-12 Russell Epstein <repst...@apple.com>
+ Revert r258354. rdar://problem/60395490
+
+2020-03-12 Russell Epstein <repst...@apple.com>
+
Revert r258037. rdar://problem/60382950
2020-03-12 Russell Epstein <repst...@apple.com>
Modified: branches/safari-610.1.7-branch/Source/WebCore/editing/TextManipulationController.cpp (258375 => 258376)
--- branches/safari-610.1.7-branch/Source/WebCore/editing/TextManipulationController.cpp 2020-03-13 00:47:11 UTC (rev 258375)
+++ branches/safari-610.1.7-branch/Source/WebCore/editing/TextManipulationController.cpp 2020-03-13 00:51:38 UTC (rev 258376)
@@ -139,11 +139,91 @@
flushPendingItemsForCallback();
}
+class ParagraphContentIterator {
+public:
+ ParagraphContentIterator(const Position& start, const Position& end)
+ : m_iterator(start, end)
+ , m_iteratorNode(m_iterator.atEnd() ? nullptr : m_iterator.range()->firstNode())
+ , m_currentNodeForFindingInvisibleContent(start.firstNode())
+ , m_pastEndNode(end.firstNode())
+ {
+ }
+
+ void advance()
+ {
+ // FIXME: Add a node callback to TextIterator instead of traversing the node tree twice like this.
+ if (m_currentNodeForFindingInvisibleContent != m_iteratorNode && m_currentNodeForFindingInvisibleContent != m_pastEndNode) {
+ moveCurrentNodeForward();
+ return;
+ }
+
+ if (m_iterator.atEnd())
+ return;
+
+ auto previousIteratorNode = m_iteratorNode;
+
+ m_iterator.advance();
+ m_iteratorNode = m_iterator.atEnd() ? nullptr : m_iterator.range()->firstNode();
+ if (previousIteratorNode != m_iteratorNode)
+ moveCurrentNodeForward();
+ }
+
+ struct CurrentContent {
+ RefPtr<Node> node;
+ StringView text;
+ bool isTextContent { false };
+ bool isReplacedContent { false };
+ };
+
+ CurrentContent currentContent()
+ {
+ CurrentContent content;
+ if (m_currentNodeForFindingInvisibleContent && m_currentNodeForFindingInvisibleContent != m_iteratorNode)
+ content = { m_currentNodeForFindingInvisibleContent.copyRef(), StringView { } };
+ else
+ content = { m_iterator.node(), m_iterator.text(), true };
+ if (content.node) {
+ if (auto* renderer = content.node->renderer()) {
+ if (renderer->isRenderReplaced()) {
+ content.isTextContent = false;
+ content.isReplacedContent = true;
+ }
+ }
+ }
+ return content;
+ }
+
+ Position startPosition()
+ {
+ return m_iterator.range()->startPosition();
+ }
+
+ Position endPosition()
+ {
+ return m_iterator.range()->endPosition();
+ }
+
+ bool atEnd() const { return m_iterator.atEnd() && m_currentNodeForFindingInvisibleContent == m_pastEndNode; }
+
+private:
+ void moveCurrentNodeForward()
+ {
+ m_currentNodeForFindingInvisibleContent = NodeTraversal::next(*m_currentNodeForFindingInvisibleContent);
+ if (!m_currentNodeForFindingInvisibleContent)
+ m_currentNodeForFindingInvisibleContent = m_pastEndNode;
+ }
+
+ TextIterator m_iterator;
+ RefPtr<Node> m_iteratorNode;
+ RefPtr<Node> m_currentNodeForFindingInvisibleContent;
+ RefPtr<Node> m_pastEndNode;
+};
+
void TextManipulationController::observeParagraphs(VisiblePosition& start, VisiblePosition& end)
{
auto document = makeRefPtr(start.deepEquivalent().document());
ASSERT(document);
- TextIterator iterator { start.deepEquivalent(), end.deepEquivalent() };
+ ParagraphContentIterator iterator { start.deepEquivalent(), end.deepEquivalent() };
if (document != start.deepEquivalent().document() || document != end.deepEquivalent().document())
return; // TextIterator's constructor may have updated the layout and executed arbitrary scripts.
@@ -150,45 +230,51 @@
ExclusionRuleMatcher exclusionRuleMatcher(m_exclusionRules);
Vector<ManipulationToken> tokensInCurrentParagraph;
Position startOfCurrentParagraph = start.deepEquivalent();
- while (!iterator.atEnd()) {
- StringView currentText = iterator.text();
-
- if (startOfCurrentParagraph.isNull())
- startOfCurrentParagraph = iterator.range()->startPosition();
-
- if (auto* currentNode = iterator.node()) {
- if (RefPtr<Element> currentElementAncestor = is<Element>(currentNode) ? downcast<Element>(currentNode) : currentNode->parentOrShadowHostElement()) {
+ for (; !iterator.atEnd(); iterator.advance()) {
+ auto content = iterator.currentContent();
+ if (content.node) {
+ if (RefPtr<Element> currentElementAncestor = is<Element>(*content.node) ? downcast<Element>(content.node.get()) : content.node->parentOrShadowHostElement()) {
if (isInManipulatedElement(*currentElementAncestor))
return; // We can exit early here because scheduleObservartionUpdate calls this function on each paragraph separately.
}
+
+ if (startOfCurrentParagraph.isNull())
+ startOfCurrentParagraph = iterator.startPosition();
}
- size_t endOfLastNewLine = 0;
+ if (content.isReplacedContent) {
+ tokensInCurrentParagraph.append(ManipulationToken { m_tokenIdentifier.generate(), "[]", true /* isExcluded */});
+ continue;
+ }
+
+ if (!content.isTextContent)
+ continue;
+
+ size_t startOfCurrentLine = 0;
size_t offsetOfNextNewLine = 0;
- while ((offsetOfNextNewLine = currentText.find('\n', endOfLastNewLine)) != notFound) {
- if (endOfLastNewLine < offsetOfNextNewLine) {
- auto stringUntilEndOfLine = currentText.substring(endOfLastNewLine, offsetOfNextNewLine - endOfLastNewLine).toString();
- tokensInCurrentParagraph.append(ManipulationToken { m_tokenIdentifier.generate(), stringUntilEndOfLine, exclusionRuleMatcher.isExcluded(iterator.node()) });
+ StringView currentText = content.text;
+ while ((offsetOfNextNewLine = currentText.find('\n', startOfCurrentLine)) != notFound) {
+ if (startOfCurrentLine < offsetOfNextNewLine) {
+ auto stringUntilEndOfLine = currentText.substring(startOfCurrentLine, offsetOfNextNewLine - startOfCurrentLine).toString();
+ tokensInCurrentParagraph.append(ManipulationToken { m_tokenIdentifier.generate(), stringUntilEndOfLine, exclusionRuleMatcher.isExcluded(content.node.get()) });
}
- auto lastRange = iterator.range();
- if (offsetOfNextNewLine < currentText.length()) {
- lastRange->setStart(firstPositionInOrBeforeNode(iterator.node())); // Move the start to the beginning of the current node.
- TextIterator::subrange(lastRange, 0, offsetOfNextNewLine);
+ if (!tokensInCurrentParagraph.isEmpty()) {
+ Position endOfCurrentParagraph = iterator.endPosition();
+ if (is<Text>(content.node)) {
+ auto& textNode = downcast<Text>(*content.node);
+ endOfCurrentParagraph = Position(&textNode, offsetOfNextNewLine);
+ startOfCurrentParagraph = Position(&textNode, offsetOfNextNewLine + 1);
+ }
+ addItem(startOfCurrentParagraph, endOfCurrentParagraph, WTFMove(tokensInCurrentParagraph));
+ startOfCurrentParagraph.clear();
}
- Position endOfCurrentParagraph = lastRange->endPosition();
-
- if (!tokensInCurrentParagraph.isEmpty())
- addItem(startOfCurrentParagraph, endOfCurrentParagraph, WTFMove(tokensInCurrentParagraph));
- startOfCurrentParagraph.clear();
- endOfLastNewLine = offsetOfNextNewLine + 1;
+ startOfCurrentLine = offsetOfNextNewLine + 1;
}
- auto remainingText = currentText.substring(endOfLastNewLine);
+ auto remainingText = currentText.substring(startOfCurrentLine);
if (remainingText.length())
- tokensInCurrentParagraph.append(ManipulationToken { m_tokenIdentifier.generate(), remainingText.toString(), exclusionRuleMatcher.isExcluded(iterator.node()) });
-
- iterator.advance();
+ tokensInCurrentParagraph.append(ManipulationToken { m_tokenIdentifier.generate(), remainingText.toString(), exclusionRuleMatcher.isExcluded(content.node.get()) });
}
if (!tokensInCurrentParagraph.isEmpty())
@@ -335,36 +421,38 @@
if (item.start.isOrphan() || item.end.isOrphan())
return ManipulationFailureType::ContentChanged;
- TextIterator iterator { item.start, item.end };
size_t currentTokenIndex = 0;
HashMap<TokenIdentifier, TokenExchangeData> tokenExchangeMap;
RefPtr<Node> commonAncestor;
- while (!iterator.atEnd()) {
- auto string = iterator.text().toString();
+ ParagraphContentIterator iterator { item.start, item.end };
+ HashSet<Ref<Node>> excludedNodes;
+ for (; !iterator.atEnd(); iterator.advance()) {
+ auto content = iterator.currentContent();
+
+ if (!content.isReplacedContent && !content.isTextContent)
+ continue;
+
if (currentTokenIndex >= item.tokens.size())
return ManipulationFailureType::ContentChanged;
+
auto& currentToken = item.tokens[currentTokenIndex];
- if (iterator.text() != currentToken.content)
+ if (!content.isReplacedContent && content.text != currentToken.content)
return ManipulationFailureType::ContentChanged;
- auto currentNode = makeRefPtr(iterator.node());
- tokenExchangeMap.set(currentToken.identifier, TokenExchangeData { currentNode.copyRef(), currentToken.content, currentToken.isExcluded });
+ tokenExchangeMap.set(currentToken.identifier, TokenExchangeData { content.node.copyRef(), currentToken.content, currentToken.isExcluded });
+ ++currentTokenIndex;
- if (currentNode) {
- // FIXME: Take care of when currentNode is nullptr.
+ // FIXME: Take care of when currentNode is nullptr.
+ if (content.node) {
if (!commonAncestor)
- commonAncestor = currentNode;
- else if (!currentNode->isDescendantOf(commonAncestor.get())) {
- commonAncestor = Range::commonAncestorContainer(commonAncestor.get(), currentNode.get());
+ commonAncestor = content.node;
+ else if (!content.node->isDescendantOf(commonAncestor.get())) {
+ commonAncestor = Range::commonAncestorContainer(commonAncestor.get(), content.node.get());
ASSERT(commonAncestor);
}
}
-
- iterator.advance();
- ++currentTokenIndex;
}
- ASSERT(commonAncestor);
RefPtr<Node> nodeAfterStart = item.start.computeNodeAfterPosition();
if (!nodeAfterStart)
@@ -398,7 +486,9 @@
exchangeData.isConsumed = true;
if (!newToken.content.isNull() && newToken.content != exchangeData.originalContent)
return ManipulationFailureType::ExclusionViolation;
- contentNode = Text::create(commonAncestor->document(), exchangeData.originalContent);
+ contentNode = exchangeData.node;
+ for (RefPtr<Node> currentDescendentNode = NodeTraversal::next(*contentNode, contentNode.get()); currentDescendentNode; currentDescendentNode = NodeTraversal::next(*currentDescendentNode, contentNode.get()))
+ nodesToRemove.remove(*currentDescendentNode);
} else
contentNode = Text::create(commonAncestor->document(), newToken.content);
@@ -436,17 +526,21 @@
}
Position insertionPoint = item.start;
+ if (insertionPoint.anchorNode() != insertionPoint.containerNode())
+ insertionPoint = insertionPoint.parentAnchoredEquivalent();
while (insertionPoint.containerNode() != commonAncestor)
insertionPoint = positionInParentBeforeNode(insertionPoint.containerNode());
ASSERT(!insertionPoint.isNull());
+ ASSERT(!insertionPoint.isOrphan());
for (auto& node : nodesToRemove)
node->remove();
for (auto& insertion : insertions) {
- if (!insertion.parentIfDifferentFromCommonAncestor)
- insertionPoint.containerNode()->insertBefore(insertion.child, insertionPoint.computeNodeBeforePosition());
- else
+ if (!insertion.parentIfDifferentFromCommonAncestor) {
+ insertionPoint.containerNode()->insertBefore(insertion.child, insertionPoint.computeNodeAfterPosition());
+ insertionPoint = positionInParentAfterNode(insertion.child.ptr());
+ } else
insertion.parentIfDifferentFromCommonAncestor->appendChild(insertion.child);
if (is<Element>(insertion.child.get()))
m_manipulatedElements.add(downcast<Element>(insertion.child.get()));
Modified: branches/safari-610.1.7-branch/Tools/ChangeLog (258375 => 258376)
--- branches/safari-610.1.7-branch/Tools/ChangeLog 2020-03-13 00:47:11 UTC (rev 258375)
+++ branches/safari-610.1.7-branch/Tools/ChangeLog 2020-03-13 00:51:38 UTC (rev 258376)
@@ -1,5 +1,9 @@
2020-03-12 Russell Epstein <repst...@apple.com>
+ Revert r258354. rdar://problem/60395490
+
+2020-03-12 Russell Epstein <repst...@apple.com>
+
Revert r258037. rdar://problem/60382950
2020-03-12 Russell Epstein <repst...@apple.com>
Modified: branches/safari-610.1.7-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm (258375 => 258376)
--- branches/safari-610.1.7-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2020-03-13 00:47:11 UTC (rev 258375)
+++ branches/safari-610.1.7-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2020-03-13 00:51:38 UTC (rev 258376)
@@ -697,6 +697,133 @@
[webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
}
+TEST(TextManipulation, CompleteTextManipulationShouldPreserveImagesAsExcludedTokens)
+{
+ auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
+ [webView _setTextManipulationDelegate:delegate.get()];
+
+ [webView synchronouslyLoadHTMLString:@"<!DOCTYPE html><html><body><div>hello, <img src="" world</div></body></html>"];
+
+ done = false;
+ [webView _startTextManipulationsWithConfiguration:nil completion:^{
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+
+ auto *items = [delegate items];
+ EXPECT_EQ(items.count, 1UL);
+ auto *tokens = items[0].tokens;
+ EXPECT_EQ(tokens.count, 3UL);
+ EXPECT_STREQ("hello, ", tokens[0].content.UTF8String);
+ EXPECT_FALSE(tokens[0].isExcluded);
+ EXPECT_STREQ("[]", tokens[1].content.UTF8String);
+ EXPECT_TRUE(tokens[1].isExcluded);
+ EXPECT_STREQ(" world", tokens[2].content.UTF8String);
+ EXPECT_FALSE(tokens[2].isExcluded);
+
+ done = false;
+ [webView _completeTextManipulationForItems:@[(_WKTextManipulationItem *)createItem(items[0].identifier, {
+ { tokens[0].identifier, @"this is " },
+ { tokens[1].identifier, nil },
+ { tokens[2].identifier, @" a test" }
+ })] completion:^(NSArray<NSError *> *errors) {
+ EXPECT_EQ(errors, nil);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+ EXPECT_WK_STREQ("<div>this is <img src="" a test</div>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
+}
+
+TEST(TextManipulation, CompleteTextManipulationShouldPreserveSVGAsExcludedTokens)
+{
+ auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
+ [webView _setTextManipulationDelegate:delegate.get()];
+
+ [webView synchronouslyLoadHTMLString:@"<!DOCTYPE html><html><body>"
+ "<section><div style=\"display: inline-block;\">hello</div>"
+ "<div style=\"display: inline-block;\"><span style=\"display: inline-flex;\">"
+ "<svg viewBox=\"0 0 20 20\" width=\"20\" height=\"20\"><rect width=\"20\" height=\"20\" fill=\"#06f\"></rect></svg>"
+ "</span></div></section><p>world</p></body></html>"];
+
+ done = false;
+ [webView _startTextManipulationsWithConfiguration:nil completion:^{
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+
+ auto *items = [delegate items];
+ EXPECT_EQ(items.count, 2UL);
+ auto *tokens = items[0].tokens;
+ EXPECT_EQ(tokens.count, 2UL);
+ EXPECT_STREQ("hello", tokens[0].content.UTF8String);
+ EXPECT_FALSE(tokens[0].isExcluded);
+ EXPECT_STREQ("[]", tokens[1].content.UTF8String);
+ EXPECT_TRUE(tokens[1].isExcluded);
+
+ EXPECT_EQ(items[1].tokens.count, 1UL);
+ EXPECT_STREQ("world", items[1].tokens[0].content.UTF8String);
+ EXPECT_FALSE(items[1].tokens[0].isExcluded);
+
+ done = false;
+ [webView _completeTextManipulationForItems:@[(_WKTextManipulationItem *)createItem(items[0].identifier, {
+ { tokens[0].identifier, @"hey" },
+ { tokens[1].identifier, nil },
+ })] completion:^(NSArray<NSError *> *errors) {
+ EXPECT_EQ(errors, nil);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+ EXPECT_WK_STREQ("<section><div style=\"display: inline-block;\">hey</div>"
+ "<div style=\"display: inline-block;\"><span style=\"display: inline-flex;\">"
+ "<svg viewBox=\"0 0 20 20\" width=\"20\" height=\"20\"><rect width=\"20\" height=\"20\" fill=\"#06f\"></rect></svg>"
+ "</span></div></section><p>world</p>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
+}
+
+TEST(TextManipulation, CompleteTextManipulationShouldPreserveOrderOfBlockImage)
+{
+ auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
+ [webView _setTextManipulationDelegate:delegate.get()];
+
+ [webView synchronouslyLoadHTMLString:@"<!DOCTYPE html><html><body><svg viewBox=\"0 0 10 10\" width=\"100\" height=\"100\">"
+ "<rect width=\"10\" height=\"10\" fill=\"red\"></rect></svg><img src=""
+ "AAAABCAIAAACQd1PeAAAAAXNSR0IArs4c6QAAAAxJREFUCNdjYKhnAAABAgCAbV7tZwAAAABJRU5ErkJggg==\""
+ "style=\"display: block; width: 100px;\"><section>helllo world</section></body></html>"];
+
+ done = false;
+ [webView _startTextManipulationsWithConfiguration:nil completion:^{
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+
+ auto *items = [delegate items];
+ EXPECT_EQ(items.count, 2UL);
+ EXPECT_EQ(items[0].tokens.count, 1UL);
+ EXPECT_STREQ("[]", items[0].tokens[0].content.UTF8String);
+ EXPECT_TRUE(items[0].tokens[0].isExcluded);
+
+ auto *tokens = items[1].tokens;
+ EXPECT_EQ(tokens.count, 1UL);
+ EXPECT_STREQ("helllo world", tokens[0].content.UTF8String);
+ EXPECT_FALSE(tokens[0].isExcluded);
+
+ done = false;
+ [webView _completeTextManipulationForItems:@[
+ (_WKTextManipulationItem *)createItem(items[0].identifier, { { items[0].tokens[0].identifier, nil } }),
+ (_WKTextManipulationItem *)createItem(items[1].identifier, { { items[1].tokens[0].identifier, @"hello, world" } }),
+ ] completion:^(NSArray<NSError *> *errors) {
+ EXPECT_EQ(errors, nil);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+ EXPECT_WK_STREQ("<svg viewBox=\"0 0 10 10\" width=\"100\" height=\"100\"><rect width=\"10\" height=\"10\" fill=\"red\"></rect></svg>"
+ "<img src=""
+ "djYKhnAAABAgCAbV7tZwAAAABJRU5ErkJggg==\" style=\"display: block; width: 100px;\"><section>hello, world</section>",
+ [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
+}
+
TEST(TextManipulation, CompleteTextManipulationShouldBatchItemCallback)
{
auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
