TextManipulationController should preserve images
https://bugs.webkit.org/show_bug.cgi?id=208692
Reviewed by Wenson Hsieh.
Source/WebCore:
This patch makes TextManipulationController preserve images (img and SVG) and their ordering,
and lays down the foundation for preserving other non-text content.
To do this, this patch introduces ParagraphContentIterator which wraps TextIterator and introduces
a secondary node iteration which detects all ndoes that have been skipped. Ideally, we would update
TextIterator to do this natively but this is tricky due to the multitude of the node traveral options
supported by TextIterator at the moment. ParagraphContentIterator::advance stops at every node
including ones that do not generate text in TextIterator. This also allows more code sharing between
TextManipulationController's observeParagraphs and replace functions.
For now, we special case nodes with RenderReplaced like img element and the root SVG element.
Tests: TextManipulation.CompleteTextManipulationShouldPreserveImagesAsExcludedTokens
TextManipulation.CompleteTextManipulationShouldPreserveSVGAsExcludedTokens
TextManipulation.CompleteTextManipulationShouldPreserveOrderOfBlockImage
* editing/TextManipulationController.cpp:
(WebCore::ParagraphContentIterator): Added.
(WebCore::ParagraphContentIterator::ParagraphContentIterator): Added.
(WebCore::ParagraphContentIterator::advance): Added.
(WebCore::ParagraphContentIterator::currentContent): Added.
(WebCore::ParagraphContentIterator::startPosition): Added.
(WebCore::ParagraphContentIterator::endPosition): Added.
(WebCore::ParagraphContentIterator::atEnd const): Added.
(WebCore::ParagraphContentIterator::moveCurrentNodeForward): Added.
(WebCore::TextManipulationController::observeParagraphs): Adopted ParagraphContentIterator. For
nodes with RenderReplaced, we generate an excluded token "[]". Removed the dead code which was
creating a subrange from TextIterator, and renamed the previously misleadingly named endOfLastNewLine
to startOfCurrentLine for clarity.
(WebCore::TextManipulationController::replace): Adopted ParagraphContentIterator. Fixed a bug that
excluded content were always assumed to be text, and a bug that the replaced content was inserted
at a wrong location when insertionPoint is a position anchored at a node being removed. Also fixed
an obvious bug that we were inserting root replaced contents at the position before a node which
appears immediately before insertionPoint instead of after; this also resulted in the replaced content
being inserted at a wrong location in some cases.
Tools:
Added tests for preserving images and their ordering.
* TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
(TextManipulation.CompleteTextManipulationShouldPreserveImagesAsExcludedTokens):
(TextManipulation.CompleteTextManipulationShouldPreserveSVGAsExcludedTokens):
(TextManipulation.CompleteTextManipulationShouldPreserveOrderOfBlockImage):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@258037 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/editing/TextManipulationController.cpp b/Source/WebCore/editing/TextManipulationController.cpp
index 140a438..09d0ecd 100644
--- a/Source/WebCore/editing/TextManipulationController.cpp
+++ b/Source/WebCore/editing/TextManipulationController.cpp
@@ -139,56 +139,142 @@
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.
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);
- }
- Position endOfCurrentParagraph = lastRange->endPosition();
-
- if (!tokensInCurrentParagraph.isEmpty())
+ 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();
- endOfLastNewLine = offsetOfNextNewLine + 1;
+ startOfCurrentParagraph.clear();
+ }
+ 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()));
