Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(741)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: Source/core/editing/ReplaceSelectionCommand.cpp

Issue 54363006: Have ReplaceSelectionCommand::InsertedNodes API take references in argument (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « Source/core/editing/ReplaceSelectionCommand.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/editing/ReplaceSelectionCommand.cpp
diff --git a/Source/core/editing/ReplaceSelectionCommand.cpp b/Source/core/editing/ReplaceSelectionCommand.cpp
index 92e69b6ba68d7ad5181e80a0085f619a30fbcb57..9fedf5aea6ec5184b2710d37e1c4cb30f71bc702 100644
--- a/Source/core/editing/ReplaceSelectionCommand.cpp
+++ b/Source/core/editing/ReplaceSelectionCommand.cpp
@@ -322,26 +322,23 @@ void ReplacementFragment::removeInterchangeNodes(Node* container)
}
}
-inline void ReplaceSelectionCommand::InsertedNodes::respondToNodeInsertion(Node* node)
+inline void ReplaceSelectionCommand::InsertedNodes::respondToNodeInsertion(Node& node)
{
- if (!node)
- return;
-
if (!m_firstNodeInserted)
- m_firstNodeInserted = node;
+ m_firstNodeInserted = &node;
- m_lastNodeInserted = node;
+ m_lastNodeInserted = &node;
}
-inline void ReplaceSelectionCommand::InsertedNodes::willRemoveNodePreservingChildren(Node* node)
+inline void ReplaceSelectionCommand::InsertedNodes::willRemoveNodePreservingChildren(Node& node)
{
if (m_firstNodeInserted == node)
- m_firstNodeInserted = NodeTraversal::next(node);
+ m_firstNodeInserted = NodeTraversal::next(&node);
if (m_lastNodeInserted == node)
- m_lastNodeInserted = node->lastChild() ? node->lastChild() : NodeTraversal::nextSkippingChildren(node);
+ m_lastNodeInserted = node.lastChild() ? node.lastChild() : NodeTraversal::nextSkippingChildren(&node);
}
-inline void ReplaceSelectionCommand::InsertedNodes::willRemoveNode(Node* node)
+inline void ReplaceSelectionCommand::InsertedNodes::willRemoveNode(Node& node)
{
if (m_firstNodeInserted == node && m_lastNodeInserted == node) {
m_firstNodeInserted = 0;
@@ -352,12 +349,12 @@ inline void ReplaceSelectionCommand::InsertedNodes::willRemoveNode(Node* node)
m_lastNodeInserted = NodeTraversal::previousSkippingChildren(m_lastNodeInserted.get());
}
-inline void ReplaceSelectionCommand::InsertedNodes::didReplaceNode(Node* node, Node* newNode)
+inline void ReplaceSelectionCommand::InsertedNodes::didReplaceNode(Node& node, Node& newNode)
{
if (m_firstNodeInserted == node)
- m_firstNodeInserted = newNode;
+ m_firstNodeInserted = &newNode;
if (m_lastNodeInserted == node)
- m_lastNodeInserted = newNode;
+ m_lastNodeInserted = &newNode;
}
ReplaceSelectionCommand::ReplaceSelectionCommand(Document& document, PassRefPtr<DocumentFragment> fragment, CommandOptions options, EditAction editAction)
@@ -482,14 +479,14 @@ void ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline(Insert
if (inlineStyle) {
if (element->isHTMLElement()) {
Vector<QualifiedName> attributes;
- HTMLElement* htmlElement = toHTMLElement(element);
+ HTMLElement& htmlElement = toHTMLElement(*element);
adamk 2013/10/31 20:10:41 This doesn't look like much of a win to me, especi
Inactive 2013/10/31 20:33:26 Done.
- if (newInlineStyle->conflictsWithImplicitStyleOfElement(htmlElement)) {
+ if (newInlineStyle->conflictsWithImplicitStyleOfElement(&htmlElement)) {
// e.g. <b style="font-weight: normal;"> is converted to <span style="font-weight: normal;">
- node = replaceElementWithSpanPreservingChildrenAndAttributes(htmlElement);
+ node = replaceElementWithSpanPreservingChildrenAndAttributes(PassRefPtr<HTMLElement>(htmlElement));
adamk 2013/10/31 20:10:41 ...this line
element = toElement(node);
- insertedNodes.didReplaceNode(htmlElement, node.get());
- } else if (newInlineStyle->extractConflictingImplicitStyleOfAttributes(htmlElement, EditingStyle::PreserveWritingDirection, 0, attributes,
+ insertedNodes.didReplaceNode(htmlElement, *node);
Inactive 2013/10/31 20:16:24 Ok, then I guess I will have to dereference here i
+ } else if (newInlineStyle->extractConflictingImplicitStyleOfAttributes(&htmlElement, EditingStyle::PreserveWritingDirection, 0, attributes,
EditingStyle::DoNotExtractMatchingStyle)) {
// e.g. <font size="3" style="font-size: 20px;"> is converted to <font style="font-size: 20px;">
for (size_t i = 0; i < attributes.size(); i++)
@@ -510,7 +507,7 @@ void ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline(Insert
if (!inlineStyle || newInlineStyle->isEmpty()) {
if (isStyleSpanOrSpanWithOnlyStyleAttribute(element) || isEmptyFontTag(element, AllowNonEmptyStyleAttribute)) {
- insertedNodes.willRemoveNodePreservingChildren(element);
+ insertedNodes.willRemoveNodePreservingChildren(*element);
removeNodePreservingChildren(element);
continue;
}
@@ -522,7 +519,7 @@ void ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline(Insert
if (isNonTableCellHTMLBlockElement(element) && areIdenticalElements(element, element->parentNode())
&& VisiblePosition(firstPositionInNode(element->parentNode())) == VisiblePosition(firstPositionInNode(element))
&& VisiblePosition(lastPositionInNode(element->parentNode())) == VisiblePosition(lastPositionInNode(element))) {
- insertedNodes.willRemoveNodePreservingChildren(element);
+ insertedNodes.willRemoveNodePreservingChildren(*element);
removeNodePreservingChildren(element);
continue;
}
@@ -534,7 +531,7 @@ void ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline(Insert
// Keep this code around for backward compatibility
if (isLegacyAppleStyleSpan(element)) {
if (!element->firstChild()) {
- insertedNodes.willRemoveNodePreservingChildren(element);
+ insertedNodes.willRemoveNodePreservingChildren(*element);
removeNodePreservingChildren(element);
continue;
}
@@ -671,7 +668,7 @@ void ReplaceSelectionCommand::removeUnrenderedTextNodesAtEnds(InsertedNodes& ins
if (lastLeafInserted && lastLeafInserted->isTextNode() && !nodeHasVisibleRenderText(toText(lastLeafInserted))
&& !enclosingNodeWithTag(firstPositionInOrBeforeNode(lastLeafInserted), selectTag)
&& !enclosingNodeWithTag(firstPositionInOrBeforeNode(lastLeafInserted), scriptTag)) {
- insertedNodes.willRemoveNode(lastLeafInserted);
+ insertedNodes.willRemoveNode(*lastLeafInserted);
removeNode(lastLeafInserted);
}
@@ -679,7 +676,7 @@ void ReplaceSelectionCommand::removeUnrenderedTextNodesAtEnds(InsertedNodes& ins
// it is a top level node in the fragment and the user can't insert into those elements.
Node* firstNodeInserted = insertedNodes.firstNodeInserted();
if (firstNodeInserted && firstNodeInserted->isTextNode() && !nodeHasVisibleRenderText(toText(firstNodeInserted))) {
- insertedNodes.willRemoveNode(firstNodeInserted);
+ insertedNodes.willRemoveNode(*firstNodeInserted);
removeNode(firstNodeInserted);
}
}
@@ -787,7 +784,7 @@ void ReplaceSelectionCommand::handleStyleSpans(InsertedNodes& insertedNodes)
style->removeBlockProperties();
if (style->isEmpty() || !wrappingStyleSpan->firstChild()) {
- insertedNodes.willRemoveNodePreservingChildren(wrappingStyleSpan);
+ insertedNodes.willRemoveNodePreservingChildren(*wrappingStyleSpan);
removeNodePreservingChildren(wrappingStyleSpan);
} else
setNodeAttribute(wrappingStyleSpan, styleAttr, style->style()->asText());
@@ -1067,6 +1064,7 @@ void ReplaceSelectionCommand::doApply()
InsertedNodes insertedNodes;
RefPtr<Node> refNode = fragment.firstChild();
+ ASSERT(refNode);
adamk 2013/10/31 20:10:41 Huh, I wonder why we know this here. The editing c
RefPtr<Node> node = refNode->nextSibling();
fragment.removeNode(refNode);
@@ -1077,7 +1075,7 @@ void ReplaceSelectionCommand::doApply()
refNode = insertAsListItems(toHTMLElement(refNode), blockStart, insertionPos, insertedNodes);
else {
insertNodeAt(refNode, insertionPos);
- insertedNodes.respondToNodeInsertion(refNode.get());
+ insertedNodes.respondToNodeInsertion(*refNode);
}
// Mutation events (bug 22634) may have already removed the inserted content
@@ -1089,8 +1087,8 @@ void ReplaceSelectionCommand::doApply()
while (node) {
RefPtr<Node> next = node->nextSibling();
fragment.removeNode(node.get());
- insertNodeAfter(node, refNode.get());
- insertedNodes.respondToNodeInsertion(node.get());
+ insertNodeAfter(node, refNode);
+ insertedNodes.respondToNodeInsertion(*node);
// Mutation events (bug 22634) may have already removed the inserted content
if (!node->inDocument())
@@ -1120,10 +1118,10 @@ void ReplaceSelectionCommand::doApply()
if (endBR && (plainTextFragment || shouldRemoveEndBR(endBR, originalVisPosBeforeEndBR))) {
RefPtr<Node> parent = endBR->parentNode();
- insertedNodes.willRemoveNode(endBR);
+ insertedNodes.willRemoveNode(*endBR);
removeNode(endBR);
if (Node* nodeToRemove = highestNodeToRemoveInPruning(parent.get())) {
- insertedNodes.willRemoveNode(nodeToRemove);
+ insertedNodes.willRemoveNode(*nodeToRemove);
removeNode(nodeToRemove);
}
}
@@ -1432,10 +1430,10 @@ Node* ReplaceSelectionCommand::insertAsListItems(PassRefPtr<HTMLElement> prpList
listElement->removeChild(listItem.get(), ASSERT_NO_EXCEPTION);
if (isStart || isMiddle) {
insertNodeBefore(listItem, lastNode);
- insertedNodes.respondToNodeInsertion(listItem.get());
+ insertedNodes.respondToNodeInsertion(*listItem);
} else if (isEnd) {
insertNodeAfter(listItem, lastNode);
- insertedNodes.respondToNodeInsertion(listItem.get());
+ insertedNodes.respondToNodeInsertion(*listItem);
lastNode = listItem.get();
} else
ASSERT_NOT_REACHED();
« no previous file with comments | « Source/core/editing/ReplaceSelectionCommand.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698