Title: [262398] trunk
Revision
262398
Author
sihui_...@apple.com
Date
2020-06-01 16:54:34 -0700 (Mon, 01 Jun 2020)

Log Message

TextManipulationController should put one Node in only one paragraph
https://bugs.webkit.org/show_bug.cgi?id=212548

Reviewed by Wenson Hsieh.

Source/WebCore:

TextManipulationController mainly uses line break as delimiter to split paragraphs. In our current
implementation, if text of a Node has line break, the part before the line break is in one paragraph and the
part after the line break is in another paragraph, which means the Node is in the ranges of two paragraphs.
In this case, when TextManipulationController manipulates the first paragraph, it replaces all the Nodes in the
range of first paragraph with new Nodes. Then when it manipulates the second paragraph, if will find Node in the
range of second paragraph does not exist and fail (because the Node is removed when handling the first
paragraph.). Also, TextManipulationController currently does not preserve line breaks in text, which can be an
issue if these line breaks are visible.

This patch makes the ParagraphContentIterator iterate over Nodes instead of text, so a Node can only be in the
range of one paragraph. To do this, it makes line break and spaces around it as a special excluded token.
Here are the rules for splitting paragraphs by line break now:
1. If the special token is the first token in a Node, text in Nodes before the Node will make a paragraph.
2. If the special token is the last token in a Node, text in Nodes before the Node and in the Node will make a
paragraph.
3. If the special token in the middle of tokens in a Node, then we don't make a new paragraph until next special
token meets condition 1 or 2.

This patch also fixes the issue that Nodes out of the paragraph range can be removed due to the preorder Node
traversal, by finding and adding those Nodes back.

* editing/TextManipulationController.cpp:
(WebCore::ParagraphContentIterator::m_pastEndNode):
(WebCore::ParagraphContentIterator::advance):
(WebCore::ParagraphContentIterator::currentContent):
(WebCore::ParagraphContentIterator::atEnd const):
(WebCore::ParagraphContentIterator::advanceNode):
(WebCore::ParagraphContentIterator::advanceIteratorNodeAndUpdateText):
(WebCore::isEnclosingItemBoundaryElement):
(WebCore::TextManipulationController::parse):
(WebCore::TextManipulationController::observeParagraphs):
(WebCore::TextManipulationController::addItem):
(WebCore::TextManipulationController::getPath):
(WebCore::TextManipulationController::updateInsertions):
(WebCore::TextManipulationController::replace):
(WebCore::ParagraphContentIterator::startPosition): Deleted.
(WebCore::ParagraphContentIterator::endPosition): Deleted.
(WebCore::ParagraphContentIterator::moveCurrentNodeForward): Deleted.
(WebCore::containsOnlyHTMLSpaces): Deleted.
* editing/TextManipulationController.h:

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (262397 => 262398)


--- trunk/Source/WebCore/ChangeLog	2020-06-01 22:19:50 UTC (rev 262397)
+++ trunk/Source/WebCore/ChangeLog	2020-06-01 23:54:34 UTC (rev 262398)
@@ -1,3 +1,51 @@
+2020-06-01  Sihui Liu  <sihui_...@apple.com>
+
+        TextManipulationController should put one Node in only one paragraph
+        https://bugs.webkit.org/show_bug.cgi?id=212548
+
+        Reviewed by Wenson Hsieh.
+
+        TextManipulationController mainly uses line break as delimiter to split paragraphs. In our current 
+        implementation, if text of a Node has line break, the part before the line break is in one paragraph and the
+        part after the line break is in another paragraph, which means the Node is in the ranges of two paragraphs.
+        In this case, when TextManipulationController manipulates the first paragraph, it replaces all the Nodes in the 
+        range of first paragraph with new Nodes. Then when it manipulates the second paragraph, if will find Node in the
+        range of second paragraph does not exist and fail (because the Node is removed when handling the first 
+        paragraph.). Also, TextManipulationController currently does not preserve line breaks in text, which can be an
+        issue if these line breaks are visible. 
+
+        This patch makes the ParagraphContentIterator iterate over Nodes instead of text, so a Node can only be in the 
+        range of one paragraph. To do this, it makes line break and spaces around it as a special excluded token.
+        Here are the rules for splitting paragraphs by line break now:
+        1. If the special token is the first token in a Node, text in Nodes before the Node will make a paragraph. 
+        2. If the special token is the last token in a Node, text in Nodes before the Node and in the Node will make a 
+        paragraph.
+        3. If the special token in the middle of tokens in a Node, then we don't make a new paragraph until next special
+        token meets condition 1 or 2.
+
+        This patch also fixes the issue that Nodes out of the paragraph range can be removed due to the preorder Node
+        traversal, by finding and adding those Nodes back.
+
+        * editing/TextManipulationController.cpp:
+        (WebCore::ParagraphContentIterator::m_pastEndNode):
+        (WebCore::ParagraphContentIterator::advance):
+        (WebCore::ParagraphContentIterator::currentContent):
+        (WebCore::ParagraphContentIterator::atEnd const):
+        (WebCore::ParagraphContentIterator::advanceNode):
+        (WebCore::ParagraphContentIterator::advanceIteratorNodeAndUpdateText):
+        (WebCore::isEnclosingItemBoundaryElement):
+        (WebCore::TextManipulationController::parse):
+        (WebCore::TextManipulationController::observeParagraphs):
+        (WebCore::TextManipulationController::addItem):
+        (WebCore::TextManipulationController::getPath):
+        (WebCore::TextManipulationController::updateInsertions):
+        (WebCore::TextManipulationController::replace):
+        (WebCore::ParagraphContentIterator::startPosition): Deleted.
+        (WebCore::ParagraphContentIterator::endPosition): Deleted.
+        (WebCore::ParagraphContentIterator::moveCurrentNodeForward): Deleted.
+        (WebCore::containsOnlyHTMLSpaces): Deleted.
+        * editing/TextManipulationController.h:
+
 2020-06-01  David Kilzer  <ddkil...@apple.com>
 
         Don't use casts to convert between WebCore::DragDestinationAction and {Web,WK}DragDestinationAction types

Modified: trunk/Source/WebCore/editing/TextManipulationController.cpp (262397 => 262398)


--- trunk/Source/WebCore/editing/TextManipulationController.cpp	2020-06-01 22:19:50 UTC (rev 262397)
+++ trunk/Source/WebCore/editing/TextManipulationController.cpp	2020-06-01 23:54:34 UTC (rev 262398)
@@ -147,28 +147,20 @@
     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_currentNodeForFindingInvisibleContent(start.firstNode())
+        , m_node(start.firstNode())
         , m_pastEndNode(end.firstNode())
     {
+        if (shouldAdvanceIteratorPastCurrentNode())
+            advanceIteratorNodeAndUpdateText();
     }
 
     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;
-        }
+        m_text = WTF::nullopt;
+        advanceNode();
 
-        if (m_iterator.atEnd())
-            return;
-
-        auto previousIteratorNode = m_iteratorNode;
-
-        m_iterator.advance();
-        m_iteratorNode = m_iterator.atEnd() ? nullptr : createLiveRange(m_iterator.range())->firstNode();
-        if (previousIteratorNode != m_iteratorNode)
-            moveCurrentNodeForward();
+        if (shouldAdvanceIteratorPastCurrentNode())
+            advanceIteratorNodeAndUpdateText();
     }
 
     struct CurrentContent {
@@ -180,11 +172,7 @@
 
     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 };
+        CurrentContent content = { m_node.copyRef(), m_text ? m_text.value() : StringView { }, !!m_text };
         if (content.node) {
             if (auto* renderer = content.node->renderer()) {
                 if (renderer->isRenderReplaced()) {
@@ -196,33 +184,39 @@
         return content;
     }
 
-    Position startPosition()
+    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; }
+
+    void advanceNode()
     {
-        return createLegacyEditingPosition(m_iterator.range().start);
-    }
+        if (m_node == m_pastEndNode)
+            return;
 
-    Position endPosition()
-    {
-        return createLegacyEditingPosition(m_iterator.range().end);
+        m_node = NodeTraversal::next(*m_node);
+        if (!m_node)
+            m_node = m_pastEndNode;
     }
 
-    bool atEnd() const { return m_iterator.atEnd() && m_currentNodeForFindingInvisibleContent == m_pastEndNode; }
-
-private:
-    void moveCurrentNodeForward()
+    void advanceIteratorNodeAndUpdateText()
     {
-        if (m_currentNodeForFindingInvisibleContent == m_pastEndNode)
-            return;
+        ASSERT(shouldAdvanceIteratorPastCurrentNode());
 
-        m_currentNodeForFindingInvisibleContent = NodeTraversal::next(*m_currentNodeForFindingInvisibleContent);
-        if (!m_currentNodeForFindingInvisibleContent)
-            m_currentNodeForFindingInvisibleContent = m_pastEndNode;
+        StringBuilder stringBuilder;
+        while (shouldAdvanceIteratorPastCurrentNode()) {
+            stringBuilder.append(m_iterator.text());
+            m_iterator.advance();
+            m_iteratorNode = m_iterator.atEnd() ? nullptr : createLiveRange(m_iterator.range())->firstNode();
+        }
+        m_text = { stringBuilder.toString() };
     }
 
     TextIterator m_iterator;
     RefPtr<Node> m_iteratorNode;
-    RefPtr<Node> m_currentNodeForFindingInvisibleContent;
+    RefPtr<Node> m_node;
     RefPtr<Node> m_pastEndNode;
+    Optional<String> m_text;
 };
 
 static bool isAttributeForTextManipulation(const Element& element, const QualifiedName& nameToCheck)
@@ -253,16 +247,6 @@
     return element.hasTagName(HTMLNames::titleTag) || element.hasTagName(HTMLNames::optionTag);
 }
 
-static bool containsOnlyHTMLSpaces(StringView text)
-{
-    for (unsigned index = 0; index < text.length(); ++index) {
-        if (isNotHTMLSpace(text[index]))
-            return false;
-    }
-
-    return true;
-}
-
 static Optional<TextManipulationController::ManipulationTokenInfo> tokenInfo(Node* node)
 {
     if (!node)
@@ -278,6 +262,83 @@
     return result;
 }
 
+static bool isEnclosingItemBoundaryElement(const Element& element)
+{
+    auto* renderer = element.renderer();
+    if (!renderer)
+        return false;
+
+    auto role = [](const Element& element) -> AccessibilityRole {
+        return AccessibilityObject::ariaRoleToWebCoreRole(element.attributeWithoutSynchronization(HTMLNames::roleAttr));
+    };
+
+    if (element.hasTagName(HTMLNames::buttonTag) || role(element) == AccessibilityRole::Button)
+        return true;
+
+    if (element.hasTagName(HTMLNames::liTag) || element.hasTagName(HTMLNames::aTag)) {
+        auto displayType = renderer->style().display();
+        if (displayType == DisplayType::Block || displayType == DisplayType::InlineBlock)
+            return true;
+
+        for (auto parent = makeRefPtr(element.parentElement()); parent; parent = parent->parentElement()) {
+            if (parent->hasTagName(HTMLNames::navTag) || role(*parent) == AccessibilityRole::LandmarkNavigation)
+                return true;
+        }
+    }
+
+    return false;
+}
+
+TextManipulationController::ManipulationTokens TextManipulationController::parse(StringView text, Node* textNode)
+{
+    Vector<ManipulationToken> tokens;
+    ExclusionRuleMatcher exclusionRuleMatcher(m_exclusionRules);
+    size_t positionOfLastNonHTMLSpace = WTF::notFound;
+    size_t startPositionOfCurrentToken = 0;
+    bool containsOnlyHTMLSpace = true;
+    bool containsLineBreak = false;
+    bool firstTokenContainsLineBreak = false;
+    bool lastTokenContainsLineBreak = false;
+
+    size_t index = 0;
+    for (; index < text.length(); ++index) {
+        auto character = text[index];
+        if (isNotHTMLSpace(character)) {
+            containsOnlyHTMLSpace = false;
+            positionOfLastNonHTMLSpace = index;
+        } else if (isHTMLLineBreak(character)) {
+            containsLineBreak = true;
+            if (positionOfLastNonHTMLSpace != WTF::notFound && startPositionOfCurrentToken <= positionOfLastNonHTMLSpace) {
+                auto tokenString = text.substring(startPositionOfCurrentToken, positionOfLastNonHTMLSpace + 1 - startPositionOfCurrentToken).toString();
+                tokens.append(ManipulationToken { m_tokenIdentifier.generate(), tokenString, tokenInfo(textNode), exclusionRuleMatcher.isExcluded(textNode) });
+                startPositionOfCurrentToken = positionOfLastNonHTMLSpace + 1;
+            }
+
+            while (index < text.length() && !isNotHTMLSpace(text[index]))
+                ++index;
+
+            --index;
+
+            auto lineBreakTokenString = text.substring(startPositionOfCurrentToken, index + 1 - startPositionOfCurrentToken).toString();
+            if (tokens.isEmpty())
+                firstTokenContainsLineBreak = true;
+            tokens.append(ManipulationToken { m_tokenIdentifier.generate(), lineBreakTokenString, tokenInfo(textNode), true });
+            startPositionOfCurrentToken = index + 1;
+            lastTokenContainsLineBreak = true;
+
+            continue;
+        }
+    }
+
+    if (startPositionOfCurrentToken < text.length()) {
+        auto tokenString = text.substring(startPositionOfCurrentToken, index + 1 - startPositionOfCurrentToken).toString();
+        tokens.append(ManipulationToken { m_tokenIdentifier.generate(), tokenString, tokenInfo(textNode), exclusionRuleMatcher.isExcluded(textNode) });
+        lastTokenContainsLineBreak = false;
+    }
+
+    return { WTFMove(tokens), containsOnlyHTMLSpace, containsLineBreak, firstTokenContainsLineBreak, lastTokenContainsLineBreak };
+}
+
 void TextManipulationController::observeParagraphs(const Position& start, const Position& end)
 {
     if (start.isNull() || end.isNull())
@@ -286,88 +347,52 @@
     auto document = makeRefPtr(start.document());
     ASSERT(document);
     ParagraphContentIterator iterator { start, end };
-    VisiblePosition visibleStart = start;
-    VisiblePosition visibleEnd = end;
+    // TextIterator's constructor may have updated the layout and executed arbitrary scripts.
     if (document != start.document() || document != end.document())
-        return; // TextIterator's constructor may have updated the layout and executed arbitrary scripts.
+        return;
 
-    ExclusionRuleMatcher exclusionRuleMatcher(m_exclusionRules);
     Vector<ManipulationToken> tokensInCurrentParagraph;
     Position startOfCurrentParagraph;
     Position endOfCurrentParagraph;
     RefPtr<Element> enclosingItemBoundaryElement;
 
-    auto isEnclosingItemBoundaryElement = [](const Element& element) {
-        auto* renderer = element.renderer();
-        if (!renderer)
-            return false;
+    for (; !iterator.atEnd(); iterator.advance()) {
+        auto content = iterator.currentContent();
+        auto* contentNode = content.node.get();
+        ASSERT(contentNode);
 
-        auto role = [](const Element& element) -> AccessibilityRole {
-            return AccessibilityObject::ariaRoleToWebCoreRole(element.attributeWithoutSynchronization(HTMLNames::roleAttr));
-        };
+        if (enclosingItemBoundaryElement && !enclosingItemBoundaryElement->contains(contentNode)) {
+            if (!tokensInCurrentParagraph.isEmpty())
+                addItem(ManipulationItemData { startOfCurrentParagraph, endOfCurrentParagraph, nullptr, nullQName(), std::exchange(tokensInCurrentParagraph, { }) });
+            enclosingItemBoundaryElement = nullptr;
+        }
 
-        if (element.hasTagName(HTMLNames::buttonTag) || role(element) == AccessibilityRole::Button)
-            return true;
-
-        if (element.hasTagName(HTMLNames::liTag) || element.hasTagName(HTMLNames::aTag)) {
-            auto displayType = renderer->style().display();
-            if (displayType == DisplayType::Block || displayType == DisplayType::InlineBlock)
-                return true;
-
-            for (auto parent = makeRefPtr(element.parentElement()); parent; parent = parent->parentElement()) {
-                if (parent->hasTagName(HTMLNames::navTag) || role(*parent) == AccessibilityRole::LandmarkNavigation)
-                    return true;
-            }
+        if (RefPtr<Element> currentElementAncestor = is<Element>(*contentNode) ? downcast<Element>(contentNode) : contentNode->parentOrShadowHostElement()) {
+            // We can exit early here because scheduleObservationUpdate calls this function on each paragraph separately.
+            if (isInManipulatedElement(*currentElementAncestor))
+                return;
         }
 
-        return false;
-    };
+        if (is<Element>(*contentNode)) {
+            auto& currentElement = downcast<Element>(*contentNode);
+            if (!content.isTextContent && canPerformTextManipulationByReplacingEntireTextContent(currentElement))
+                addItem(ManipulationItemData { Position(), Position(), makeWeakPtr(currentElement), nullQName(), { ManipulationToken { m_tokenIdentifier.generate(), currentElement.textContent(), tokenInfo(&currentElement) } } });
 
-    for (; !iterator.atEnd(); iterator.advance()) {
-        auto content = iterator.currentContent();
-        if (content.node) {
-            if (enclosingItemBoundaryElement && !enclosingItemBoundaryElement->contains(content.node.get())) {
-                if (!tokensInCurrentParagraph.isEmpty())
-                    addItem(ManipulationItemData { startOfCurrentParagraph, endOfCurrentParagraph, nullptr, nullQName(), std::exchange(tokensInCurrentParagraph, { }) });
-                enclosingItemBoundaryElement = nullptr;
-            }
-
-            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 (is<Element>(*content.node)) {
-                auto& currentElement = downcast<Element>(*content.node);
-                if (!content.isTextContent && canPerformTextManipulationByReplacingEntireTextContent(currentElement)) {
-                    addItem(ManipulationItemData { Position(), Position(), makeWeakPtr(currentElement), nullQName(),
-                        { ManipulationToken { m_tokenIdentifier.generate(), currentElement.textContent(), tokenInfo(&currentElement) } } });
+            if (currentElement.hasAttributes()) {
+                for (auto& attribute : currentElement.attributesIterator()) {
+                    if (isAttributeForTextManipulation(currentElement, attribute.name()))
+                        addItem(ManipulationItemData { Position(), Position(), makeWeakPtr(currentElement), attribute.name(), { ManipulationToken { m_tokenIdentifier.generate(), attribute.value(), tokenInfo(&currentElement) } } });
                 }
-                if (currentElement.hasAttributes()) {
-                    for (auto& attribute : currentElement.attributesIterator()) {
-                        if (isAttributeForTextManipulation(currentElement, attribute.name())) {
-                            addItem(ManipulationItemData { Position(), Position(), makeWeakPtr(currentElement), attribute.name(),
-                                { ManipulationToken { m_tokenIdentifier.generate(), attribute.value(), tokenInfo(&currentElement) } } });
-                        }
-                    }
-                }
-                if (!enclosingItemBoundaryElement && isEnclosingItemBoundaryElement(currentElement))
-                    enclosingItemBoundaryElement = &currentElement;
             }
+            if (!enclosingItemBoundaryElement && isEnclosingItemBoundaryElement(currentElement))
+                enclosingItemBoundaryElement = &currentElement;
         }
 
         if (content.isReplacedContent) {
-            if (tokensInCurrentParagraph.isEmpty())
-                continue;
-
-            auto currentEndOfCurrentParagraph = positionAfterNode(content.node.get());
-            // This is at the same Node as last token, so it is already included in current range.
-            if (!is<Text>(content.node) && currentEndOfCurrentParagraph.equals(endOfCurrentParagraph))
-                continue;
-
-            endOfCurrentParagraph = currentEndOfCurrentParagraph;
-            tokensInCurrentParagraph.append(ManipulationToken { m_tokenIdentifier.generate(), "[]", tokenInfo(content.node.get()), true });
-
+            if (!tokensInCurrentParagraph.isEmpty()) {
+                tokensInCurrentParagraph.append(ManipulationToken { m_tokenIdentifier.generate(), "[]", tokenInfo(content.node.get()), true });
+                endOfCurrentParagraph = positionAfterNode(contentNode);
+            }
             continue;
         }
 
@@ -374,38 +399,22 @@
         if (!content.isTextContent)
             continue;
 
-        size_t startOfCurrentLine = 0;
-        size_t offsetOfNextNewLine = 0;
-        StringView currentText = content.text;
-        while ((offsetOfNextNewLine = currentText.find('\n', startOfCurrentLine)) != notFound) {
-            if (is<Text>(content.node) && startOfCurrentLine < offsetOfNextNewLine) {
-                auto stringUntilEndOfLine = currentText.substring(startOfCurrentLine, offsetOfNextNewLine - startOfCurrentLine).toString();
-                auto& textNode = downcast<Text>(*content.node);
-                endOfCurrentParagraph = Position(&textNode, offsetOfNextNewLine);
-                if (tokensInCurrentParagraph.isEmpty())
-                    startOfCurrentParagraph = Position(&textNode, startOfCurrentLine);
+        auto tokensInCurrentNode = parse(content.text, contentNode);
+        if (!tokensInCurrentParagraph.isEmpty() && tokensInCurrentNode.firstTokenContainsLineBreak)
+            addItem(ManipulationItemData { startOfCurrentParagraph, endOfCurrentParagraph, nullptr, nullQName(), std::exchange(tokensInCurrentParagraph, { }) });
 
-                tokensInCurrentParagraph.append(ManipulationToken { m_tokenIdentifier.generate(), stringUntilEndOfLine, tokenInfo(&textNode), exclusionRuleMatcher.isExcluded(content.node.get()) });
-            }
-
-            if (!tokensInCurrentParagraph.isEmpty()) {
-                if (is<HTMLBRElement>(content.node))
-                    endOfCurrentParagraph = positionAfterNode(content.node.get());
-                addItem(ManipulationItemData { startOfCurrentParagraph, endOfCurrentParagraph, nullptr, nullQName(), std::exchange(tokensInCurrentParagraph, { }) });
-            }
-            startOfCurrentLine = offsetOfNextNewLine + 1;
+        if (tokensInCurrentParagraph.isEmpty()) {
+            if (tokensInCurrentNode.containsOnlyHTMLSpace)
+                continue;
+            startOfCurrentParagraph = firstPositionInOrBeforeNode(contentNode);
         }
 
-        auto remainingText = currentText.substring(startOfCurrentLine);
-        if (!containsOnlyHTMLSpaces(remainingText)) {
-            if (tokensInCurrentParagraph.isEmpty()) {
-                if (startOfCurrentLine && is<Text>(content.node))
-                    startOfCurrentParagraph = Position(&downcast<Text>(*content.node), startOfCurrentLine);
-                else
-                    startOfCurrentParagraph = iterator.startPosition();
-            }
-            endOfCurrentParagraph = iterator.endPosition();
-            tokensInCurrentParagraph.append(ManipulationToken { m_tokenIdentifier.generate(), remainingText.toString(), tokenInfo(content.node.get()), exclusionRuleMatcher.isExcluded(content.node.get()) });
+        tokensInCurrentParagraph.appendVector(tokensInCurrentNode.tokens);
+        endOfCurrentParagraph = positionAfterNode(contentNode);
+
+        if (!tokensInCurrentParagraph.isEmpty() && tokensInCurrentNode.lastTokenContainsLineBreak) {
+            ASSERT(!tokensInCurrentParagraph.isEmpty());
+            addItem(ManipulationItemData { startOfCurrentParagraph, endOfCurrentParagraph, nullptr, nullQName(), std::exchange(tokensInCurrentParagraph, { }) });
         }
     }
 
@@ -489,6 +498,7 @@
     const unsigned itemCallbackBatchingSize = 128;
 
     ASSERT(m_document);
+    ASSERT(!itemData.tokens.isEmpty());
     auto newID = m_itemIdentifier.generate();
     m_pendingItemsForCallback.append(ManipulationItem {
         newID,
@@ -546,19 +556,48 @@
     String newData;
 };
 
-struct NodeInsertion {
-    RefPtr<Node> parentIfDifferentFromCommonAncestor;
-    Ref<Node> child;
-};
+Vector<Ref<Node>> TextManipulationController::getPath(Node* ancestor, Node* node)
+{
+    Vector<Ref<Node>> path;
+    RefPtr<ContainerNode> containerNode = is<ContainerNode>(*node) ? &downcast<ContainerNode>(*node) : node->parentNode();
+    for (; containerNode && containerNode != ancestor; containerNode = containerNode->parentNode())
+        path.append(*containerNode);
+    path.reverse();
+    return path;
+}
 
+void TextManipulationController::updateInsertions(Vector<NodeEntry>& lastTopDownPath, const Vector<Ref<Node>>& currentTopDownPath, Node* currentNode, HashSet<Ref<Node>>& insertedNodes, Vector<NodeInsertion>& insertions)
+{
+    size_t i =0;
+    while (i < lastTopDownPath.size() && i < currentTopDownPath.size() && lastTopDownPath[i].first.ptr() == currentTopDownPath[i].ptr())
+        ++i;
+
+    if (i != lastTopDownPath.size() || i != currentTopDownPath.size()) {
+        if (i < lastTopDownPath.size())
+            lastTopDownPath.shrink(i);
+
+        for (;i < currentTopDownPath.size(); ++i) {
+            Ref<Node> node = currentTopDownPath[i];
+            if (!insertedNodes.add(node.copyRef()).isNewEntry) {
+                auto clonedNode = node->cloneNodeInternal(node->document(), Node::CloningOperation::OnlySelf);
+                if (auto* data = ""
+                    data->eventListenerMap.copyEventListenersNotCreatedFromMarkupToTarget(clonedNode.ptr());
+                node = WTFMove(clonedNode);
+            }
+            insertions.append(NodeInsertion { lastTopDownPath.size() ? lastTopDownPath.last().second.ptr() : nullptr, node.copyRef() });
+            lastTopDownPath.append({ currentTopDownPath[i].copyRef(), WTFMove(node) });
+        }
+    }
+
+    if (currentNode)
+        insertions.append(NodeInsertion { lastTopDownPath.size() ? lastTopDownPath.last().second.ptr() : nullptr, *currentNode });
+}
+
 auto TextManipulationController::replace(const ManipulationItemData& item, const Vector<ManipulationToken>& replacementTokens) -> Optional<ManipulationFailureType>
 {
     if (item.start.isOrphan() || item.end.isOrphan())
         return ManipulationFailureType::ContentChanged;
 
-    size_t currentTokenIndex = 0;
-    HashMap<TokenIdentifier, TokenExchangeData> tokenExchangeMap;
-
     if (item.start.isNull() || item.end.isNull()) {
         RELEASE_ASSERT(item.tokens.size() == 1);
         auto element = makeRefPtr(item.element.get());
@@ -582,123 +621,114 @@
         return WTF::nullopt;
     }
 
+    size_t currentTokenIndex = 0;
+    HashMap<TokenIdentifier, TokenExchangeData> tokenExchangeMap;
     RefPtr<Node> commonAncestor;
     RefPtr<Node> firstContentNode;
+    RefPtr<Node> lastChildOfCommonAncestorInRange;
+    HashSet<Ref<Node>> nodesToRemove;
     ParagraphContentIterator iterator { item.start, item.end };
-    HashSet<Ref<Node>> excludedNodes;
-    HashSet<Ref<Node>> nodesToRemove;
-    RefPtr<Node> nodeToInsertBackAtEnd;
     for (; !iterator.atEnd(); iterator.advance()) {
         auto content = iterator.currentContent();
-        
-        if (content.node)
-            nodesToRemove.add(*content.node);
+        ASSERT(content.node);
 
+        lastChildOfCommonAncestorInRange = content.node;
+        nodesToRemove.add(*content.node);
+
         if (!content.isReplacedContent && !content.isTextContent)
             continue;
 
-        if (currentTokenIndex >= item.tokens.size()) {
-            if (content.node && !nodeToInsertBackAtEnd && content.isTextContent && content.text == "\n") { // br
-                nodeToInsertBackAtEnd = content.node;
-                continue;
-            }
-            return ManipulationFailureType::ContentChanged;
-        }
+        Vector<ManipulationToken> tokensInCurrentNode;
+        if (content.isReplacedContent)
+            tokensInCurrentNode.append(item.tokens[currentTokenIndex]);
+        else
+            tokensInCurrentNode = parse(content.text, content.node.get()).tokens;
 
-        if (content.isTextContent && containsOnlyHTMLSpaces(content.text)) {
-            // <br> should not exist in the middle of a paragraph.
-            if (is<HTMLBRElement>(content.node))
+        bool isNodeIncluded = WTF::anyOf(tokensInCurrentNode, [] (auto& token) {
+            return !token.isExcluded;
+        });
+        for (auto& token : tokensInCurrentNode) {
+            if (currentTokenIndex > item.tokens.size())
                 return ManipulationFailureType::ContentChanged;
-            continue;
+
+            auto& currentToken = item.tokens[currentTokenIndex++];
+            if (!content.isReplacedContent && currentToken.content != token.content)
+                return ManipulationFailureType::ContentChanged;
+
+            tokenExchangeMap.set(currentToken.identifier, TokenExchangeData { content.node.copyRef(), currentToken.content, !isNodeIncluded });
         }
 
-        auto& currentToken = item.tokens[currentTokenIndex];
-        if (!content.isReplacedContent && content.text != currentToken.content)
-            return ManipulationFailureType::ContentChanged;
-
-        tokenExchangeMap.set(currentToken.identifier, TokenExchangeData { content.node.copyRef(), currentToken.content, currentToken.isExcluded });
-        ++currentTokenIndex;
-        
         if (!firstContentNode)
             firstContentNode = content.node;
 
-        // FIXME: Take care of when currentNode is nullptr.
-        if (RefPtr<Node> parent = content.node ? content.node->parentNode() : nullptr) {
-            if (!commonAncestor)
-                commonAncestor = parent;
-            else if (!parent->isDescendantOf(commonAncestor.get())) {
-                commonAncestor = commonInclusiveAncestor(*commonAncestor, *parent);
-                ASSERT(commonAncestor);
-            }
+        auto parentNode = content.node->parentNode();
+        if (!commonAncestor)
+            commonAncestor = parentNode;
+        else if (!parentNode->isDescendantOf(commonAncestor.get())) {
+            commonAncestor = commonInclusiveAncestor(*commonAncestor, *parentNode);
+            ASSERT(commonAncestor);
         }
     }
 
+    while (lastChildOfCommonAncestorInRange && lastChildOfCommonAncestorInRange->parentNode() != commonAncestor)
+        lastChildOfCommonAncestorInRange = lastChildOfCommonAncestorInRange->parentNode();
+
     for (auto node = commonAncestor; node; node = node->parentNode())
         nodesToRemove.remove(*node);
 
-    Vector<Ref<Node>> currentElementStack;
     HashSet<Ref<Node>> reusedOriginalNodes;
+    Vector<NodeEntry> lastTopDownPath;
     Vector<NodeInsertion> insertions;
-    for (auto& newToken : replacementTokens) {
-        auto it = tokenExchangeMap.find(newToken.identifier);
+    for (size_t index = 0; index < replacementTokens.size(); ++index) {
+        auto& replacementToken = replacementTokens[index];
+        auto it = tokenExchangeMap.find(replacementToken.identifier);
         if (it == tokenExchangeMap.end())
             return ManipulationFailureType::InvalidToken;
 
         auto& exchangeData = it->value;
+        auto* originalNode = exchangeData.node.get();
+        ASSERT(originalNode);
+        auto replacementText = replacementToken.content;
 
-        RefPtr<Node> contentNode;
+        RefPtr<Node> replacementNode;
         if (exchangeData.isExcluded) {
             if (exchangeData.isConsumed)
                 return ManipulationFailureType::ExclusionViolation;
             exchangeData.isConsumed = true;
-            if (!newToken.content.isNull() && newToken.content != exchangeData.originalContent)
+
+            if (!replacementToken.content.isNull() && replacementToken.content != exchangeData.originalContent)
                 return ManipulationFailureType::ExclusionViolation;
-            contentNode = exchangeData.node;
-            for (RefPtr<Node> currentDescendentNode = NodeTraversal::next(*contentNode, contentNode.get()); currentDescendentNode; currentDescendentNode = NodeTraversal::next(*currentDescendentNode, contentNode.get()))
-                nodesToRemove.remove(*currentDescendentNode);
+
+            replacementNode = originalNode;
+            for (RefPtr<Node> descendentNode = NodeTraversal::next(*originalNode, originalNode); descendentNode; descendentNode = NodeTraversal::next(*descendentNode, originalNode))
+                nodesToRemove.remove(*descendentNode);
         } else
-            contentNode = Text::create(commonAncestor->document(), newToken.content);
+            replacementNode = Text::create(commonAncestor->document(), replacementText);
 
-        auto& originalNode = exchangeData.node ? *exchangeData.node : *commonAncestor;
-        RefPtr<ContainerNode> currentNode = is<ContainerNode>(originalNode) ? &downcast<ContainerNode>(originalNode) : originalNode.parentNode();
+        auto topDownPath = getPath(commonAncestor.get(), originalNode);
+        updateInsertions(lastTopDownPath, topDownPath, replacementNode.get(), reusedOriginalNodes, insertions);
+    }
 
-        Vector<Ref<Node>> currentAncestors;
-        for (; currentNode && currentNode != commonAncestor; currentNode = currentNode->parentNode())
-            currentAncestors.append(*currentNode);
-        currentAncestors.reverse();
+    auto end = lastChildOfCommonAncestorInRange ? positionInParentAfterNode(lastChildOfCommonAncestorInRange.get()) : positionAfterNode(commonAncestor.get());
+    RefPtr<Node> node = item.end.firstNode();
+    RefPtr<Node> endNode = end.firstNode();
+    if (node && node != endNode) {
+        auto topDownPath = getPath(commonAncestor.get(), node->parentNode());
+        updateInsertions(lastTopDownPath, topDownPath, nullptr, reusedOriginalNodes, insertions);
+    }
+    while (node != endNode) {
+        Ref<Node> parentNode = *node->parentNode();
+        while (!lastTopDownPath.isEmpty() && lastTopDownPath.last().first.ptr() != parentNode.ptr())
+            lastTopDownPath.removeLast();
 
-        size_t i =0;
-        while (i < currentElementStack.size() && i < currentAncestors.size() && currentElementStack[i].ptr() == currentAncestors[i].ptr())
-            ++i;
-
-        if (i == currentElementStack.size() && i == currentAncestors.size())
-            insertions.append(NodeInsertion { currentElementStack.size() ? currentElementStack.last().ptr() : nullptr, contentNode.releaseNonNull() });
-        else {
-            if (i < currentElementStack.size())
-                currentElementStack.shrink(i);
-            for (;i < currentAncestors.size(); ++i) {
-                Ref<Node> currentNode = currentAncestors[i].copyRef();
-                if (!reusedOriginalNodes.add(currentNode.copyRef()).isNewEntry) {
-                    auto clonedNode = currentNode->cloneNodeInternal(currentNode->document(), Node::CloningOperation::OnlySelf);
-                    if (auto* data = ""
-                        data->eventListenerMap.copyEventListenersNotCreatedFromMarkupToTarget(clonedNode.ptr());
-                    currentNode = WTFMove(clonedNode);
-                }
-
-                insertions.append(NodeInsertion { currentElementStack.size() ? currentElementStack.last().ptr() : nullptr, currentNode.copyRef() });
-                currentElementStack.append(WTFMove(currentNode));
-            }
-            insertions.append(NodeInsertion { currentElementStack.size() ? currentElementStack.last().ptr() : nullptr, contentNode.releaseNonNull() });
-        }
+        insertions.append(NodeInsertion { lastTopDownPath.size() ? lastTopDownPath.last().second.ptr() : nullptr, *node });
+        lastTopDownPath.append({ *node, *node });
+        node = NodeTraversal::next(*node);
     }
-    if (nodeToInsertBackAtEnd)
-        insertions.append(NodeInsertion { nullptr, nodeToInsertBackAtEnd.releaseNonNull() });
 
     Position insertionPoint = positionBeforeNode(firstContentNode.get()).parentAnchoredEquivalent();
     while (insertionPoint.containerNode() != commonAncestor)
         insertionPoint = positionInParentBeforeNode(insertionPoint.containerNode());
-    ASSERT(!insertionPoint.isNull());
-    ASSERT(!insertionPoint.isOrphan());
 
     for (auto& node : nodesToRemove)
         node->remove();

Modified: trunk/Source/WebCore/editing/TextManipulationController.h (262397 => 262398)


--- trunk/Source/WebCore/editing/TextManipulationController.h	2020-06-01 22:19:50 UTC (rev 262397)
+++ trunk/Source/WebCore/editing/TextManipulationController.h	2020-06-01 23:54:34 UTC (rev 262398)
@@ -150,8 +150,25 @@
         Vector<ManipulationToken> tokens;
     };
 
+    struct ManipulationTokens {
+        Vector<ManipulationToken> tokens;
+        bool containsOnlyHTMLSpace { true };
+        bool containsLineBreak { false };
+        bool firstTokenContainsLineBreak { false };
+        bool lastTokenContainsLineBreak { false };
+    };
+    ManipulationTokens parse(StringView, Node*);
+
     void addItem(ManipulationItemData&&);
     void flushPendingItemsForCallback();
+
+    struct NodeInsertion {
+        RefPtr<Node> parentIfDifferentFromCommonAncestor;
+        Ref<Node> child;
+    };
+    using NodeEntry = std::pair<Ref<Node>, Ref<Node>>;
+    Vector<Ref<Node>> getPath(Node*, Node*);
+    void updateInsertions(Vector<NodeEntry>&, const Vector<Ref<Node>>&, Node*, HashSet<Ref<Node>>&, Vector<NodeInsertion>&);
     Optional<ManipulationFailureType> replace(const ManipulationItemData&, const Vector<ManipulationToken>&);
 
     WeakPtr<Document> m_document;

Modified: trunk/Tools/ChangeLog (262397 => 262398)


--- trunk/Tools/ChangeLog	2020-06-01 22:19:50 UTC (rev 262397)
+++ trunk/Tools/ChangeLog	2020-06-01 23:54:34 UTC (rev 262398)
@@ -1,3 +1,13 @@
+2020-06-01  Sihui Liu  <sihui_...@apple.com>
+
+        TextManipulationController should put one Node in only one paragraph
+        https://bugs.webkit.org/show_bug.cgi?id=212548
+
+        Reviewed by Wenson Hsieh.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
+        (TestWebKitAPI::TEST):
+
 2020-06-01  Alex Christensen  <achristen...@webkit.org>
 
         Make CustomDisplayName and DefaultDisplayName API tests fail instead of timing out when something changes

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm (262397 => 262398)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm	2020-06-01 22:19:50 UTC (rev 262397)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm	2020-06-01 23:54:34 UTC (rev 262398)
@@ -179,15 +179,13 @@
     TestWebKitAPI::Util::run(&done);
 
     auto *items = [delegate items];
-    EXPECT_EQ(items.count, 3UL);
-    EXPECT_EQ(items[0].tokens.count, 1UL);
+    EXPECT_EQ(items.count, 1UL);
+    EXPECT_EQ(items[0].tokens.count, 5UL);
     EXPECT_STREQ("hello", items[0].tokens[0].content.UTF8String);
-
-    EXPECT_EQ(items[1].tokens.count, 1UL);
-    EXPECT_STREQ("world", items[1].tokens[0].content.UTF8String);
-
-    EXPECT_EQ(items[2].tokens.count, 1UL);
-    EXPECT_STREQ("WebKit", items[2].tokens[0].content.UTF8String);
+    EXPECT_STREQ("\n", items[0].tokens[1].content.UTF8String);
+    EXPECT_STREQ("world", items[0].tokens[2].content.UTF8String);
+    EXPECT_STREQ("\n", items[0].tokens[3].content.UTF8String);
+    EXPECT_STREQ("WebKit", items[0].tokens[4].content.UTF8String);
 }
 
 TEST(TextManipulation, StartTextManipulationFindParagraphsWithMultileTokens)
@@ -786,6 +784,71 @@
     EXPECT_WK_STREQ("Two", items[1].tokens[0].content);
 }
 
+TEST(TextManipulation, StartTextManipulationExtractsVisibleLineBreaksInTextAsExcludedTokens)
+{
+    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>"
+        "<body>"
+        "<span style='white-space:pre;'>&#10; one&#10;  two</span>"
+        "<span style='white-space:pre;'>&#10;</span>"
+        "<span>&#10; three&#10;  four&#10;</span>"
+        "</body>"];
+
+    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, 4UL);
+    EXPECT_STREQ("\n ", items[0].tokens[0].content.UTF8String);
+    EXPECT_TRUE(items[0].tokens[0].isExcluded);
+    EXPECT_STREQ("one", items[0].tokens[1].content.UTF8String);
+    EXPECT_FALSE(items[0].tokens[1].isExcluded);
+    EXPECT_STREQ("\n  ", items[0].tokens[2].content.UTF8String);
+    EXPECT_TRUE(items[0].tokens[2].isExcluded);
+    EXPECT_STREQ("two", items[0].tokens[3].content.UTF8String);
+    EXPECT_FALSE(items[0].tokens[3].isExcluded);
+    EXPECT_EQ(items[1].tokens.count, 1UL);
+    EXPECT_STREQ("three four", items[1].tokens[0].content.UTF8String);
+}
+
+TEST(TextManipulation, StartTextManipulationExtractsValuesByNode)
+{
+    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>"
+        "<body>"
+        "<span>one</span><span style='white-space:pre;'>two &#10; three</span><span>four</span>"
+        "<span style='white-space:pre;'>&#10;five</span>"
+        "</body>"];
+
+    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, 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);
+}
+
 struct Token {
     NSString *identifier;
     NSString *content;
@@ -1064,7 +1127,7 @@
         done = true;
     }];
     TestWebKitAPI::Util::run(&done);
-    EXPECT_WK_STREQ("<p><b>Hello, </b>World<br>WebKit</p>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
+    EXPECT_WK_STREQ("<p><b>Hello, </b>World<b><br></b>WebKit</p>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
 }
 
 TEST(TextManipulation, CompleteTextManipulationFailWhenBRIsInserted)
@@ -1967,6 +2030,123 @@
     EXPECT_WK_STREQ("<div class=\"frame\"><div class=\"subframe\"></div></div><style></style><div class=\"inline\"><div><li><a href="" href="" class=\"frame\"><div class=\"subframe\"></div></div>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
 }
 
+TEST(TextManipulation, CompleteTextManipulationCanMergeContentAndPreserveLineBreaks)
+{
+    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>"
+        "<body>"
+        "<span style='white-space:pre;'>one &#10; two &#10;</span>"
+        "</body>"];
+
+    done = false;
+    [webView _startTextManipulationsWithConfiguration:nil completion:^{
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+
+    auto items = [delegate items];
+    EXPECT_EQ(items.count, 1UL);
+    EXPECT_EQ(items[0].tokens.count, 4UL);
+    EXPECT_STREQ("one", items[0].tokens[0].content.UTF8String);
+    EXPECT_STREQ(" \n ", items[0].tokens[1].content.UTF8String);
+    EXPECT_STREQ("two", items[0].tokens[2].content.UTF8String);
+    EXPECT_STREQ(" \n", items[0].tokens[3].content.UTF8String);
+
+    auto replacementItems = retainPtr(@[
+        createItem(items[0].identifier, { { items[0].tokens[0].identifier, @"ONE" }, { items[0].tokens[1].identifier, items[0].tokens[1].content }, { items[0].tokens[2].identifier, @"TWO" }, { items[0].tokens[3].identifier, items[0].tokens[3].content } }).autorelease(),
+    ]);
+
+    done = false;
+    [webView _completeTextManipulationForItems:replacementItems.get() completion:^(NSArray<NSError *> *errors) {
+        EXPECT_EQ(errors, nil);
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+
+    EXPECT_WK_STREQ("<span style=\"white-space:pre;\">ONE \n TWO \n</span>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
+}
+
+TEST(TextManipulation, CompleteTextManipulationShouldPreserveNodesAfterParagraphRange)
+{
+    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>"
+        "<body>"
+        "<p>hello<b>world<br><br><i>from</i></b></p>"
+        "</body>"];
+
+    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, 2UL);
+    EXPECT_STREQ("hello", items[0].tokens[0].content.UTF8String);
+    EXPECT_STREQ("world", items[0].tokens[1].content.UTF8String);
+    EXPECT_EQ(items[1].tokens.count, 1UL);
+    EXPECT_STREQ("from", items[1].tokens[0].content.UTF8String);
+
+    auto replacementItems = retainPtr(@[
+        createItem(items[0].identifier, { { items[0].tokens[0].identifier, @"Hello" }, { items[0].tokens[1].identifier, @"World" } }).autorelease(),
+        createItem(items[1].identifier, { { items[1].tokens[0].identifier, @"From WebKit" } }).autorelease(),
+    ]);
+
+    done = false;
+    [webView _completeTextManipulationForItems:replacementItems.get() completion:^(NSArray<NSError *> *errors) {
+        EXPECT_EQ(errors, nil);
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+
+    EXPECT_WK_STREQ("<p>Hello<b>World<br><br><i>From WebKit</i></b></p>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
+}
+
+TEST(TextManipulation, CompleteTextManipulationSPreserveNodesBeforeParagraphRange)
+{
+    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>"
+        "<body>"
+        "<p><b><br><br>hello<i>world</i></b>from</p>"
+        "</body>"];
+
+    done = false;
+    [webView _startTextManipulationsWithConfiguration:nil completion:^{
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+
+    auto items = [delegate items];
+    EXPECT_EQ(items.count, 1UL);
+    EXPECT_EQ(items[0].tokens.count, 3UL);
+    EXPECT_STREQ("hello", items[0].tokens[0].content.UTF8String);
+    EXPECT_STREQ("world", items[0].tokens[1].content.UTF8String);
+    EXPECT_STREQ("from", items[0].tokens[2].content.UTF8String);
+
+    auto replacementItems = retainPtr(@[
+        createItem(items[0].identifier, { { items[0].tokens[0].identifier, @"Hello" }, { items[0].tokens[1].identifier, @"World" }, { items[0].tokens[2].identifier, @"From WebKit" } }).autorelease()
+    ]);
+
+    done = false;
+    [webView _completeTextManipulationForItems:replacementItems.get() completion:^(NSArray<NSError *> *errors) {
+        EXPECT_EQ(errors, nil);
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+
+    EXPECT_WK_STREQ("<p><b><br><br>Hello<i>World</i></b>From WebKit</p>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
+}
+
 TEST(TextManipulation, InsertingContentIntoAlreadyManipulatedContentDoesNotCreateTextManipulationItem)
 {
     auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to