2009-03-12 Julien Chaffraix <jchaffraix@webkit.org>
Reviewed by Darin Adler.
Bug 24110: cloneNode should call cloneElement and not the reverse
- Splitted the code from cloneNode into cloneElementWithChildren and cloneElementWithChildren.
Now cloneNode calls one of the 2 previous methods.
- Renamed cloneElement to cloneElementWithoutChildren as it was the previous behaviour.
- Moved cloneNode to the Element private section so that WebCore callers cannot use it.
- Removed Element::cloneNode usage through WebCore.
* dom/Element.cpp:
(WebCore::Element::cloneNode): Moved to Element's private section and it
now calls the two next methods.
(WebCore::Element::cloneElementWithChildren): Added.
(WebCore::Element::cloneElementWithoutChildren): Renamed from cloneElement
to avoid ambiguity.
* dom/Element.h:
* editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::addInlineStyleIfNeeded): Changed call to cloneElement
to call to cloneElementWithoutChildren.
* editing/BreakBlockquoteCommand.cpp:
(WebCore::BreakBlockquoteCommand::doApply): Ditto.
* editing/IndentOutdentCommand.cpp:
(WebCore::IndentOutdentCommand::indentRegion): Ditto.
* editing/InsertParagraphSeparatorCommand.cpp:
(WebCore::InsertParagraphSeparatorCommand::doApply): Ditto.
* editing/ModifySelectionListLevel.cpp:
(WebCore::IncreaseSelectionListLevelCommand::doApply): Ditto.
* editing/SplitElementCommand.cpp:
(WebCore::SplitElementCommand::doApply): Ditto.
* editing/markup.cpp:
(WebCore::createFragmentFromText): Ditto.
* svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::buildShadowTree): Ditto.
(WebCore::SVGUseElement::expandUseElementsInShadowTree): Ditto.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@41621 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index c389fad..7ec8d11 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,45 @@
+2009-03-12 Julien Chaffraix <jchaffraix@webkit.org>
+
+ Reviewed by Darin Adler.
+
+ Bug 24110: cloneNode should call cloneElement and not the reverse
+
+ - Splitted the code from cloneNode into cloneElementWithChildren and cloneElementWithChildren.
+ Now cloneNode calls one of the 2 previous methods.
+
+ - Renamed cloneElement to cloneElementWithoutChildren as it was the previous behaviour.
+
+ - Moved cloneNode to the Element private section so that WebCore callers cannot use it.
+
+ - Removed Element::cloneNode usage through WebCore.
+
+ * dom/Element.cpp:
+ (WebCore::Element::cloneNode): Moved to Element's private section and it
+ now calls the two next methods.
+ (WebCore::Element::cloneElementWithChildren): Added.
+ (WebCore::Element::cloneElementWithoutChildren): Renamed from cloneElement
+ to avoid ambiguity.
+ * dom/Element.h:
+
+ * editing/ApplyStyleCommand.cpp:
+ (WebCore::ApplyStyleCommand::addInlineStyleIfNeeded): Changed call to cloneElement
+ to call to cloneElementWithoutChildren.
+ * editing/BreakBlockquoteCommand.cpp:
+ (WebCore::BreakBlockquoteCommand::doApply): Ditto.
+ * editing/IndentOutdentCommand.cpp:
+ (WebCore::IndentOutdentCommand::indentRegion): Ditto.
+ * editing/InsertParagraphSeparatorCommand.cpp:
+ (WebCore::InsertParagraphSeparatorCommand::doApply): Ditto.
+ * editing/ModifySelectionListLevel.cpp:
+ (WebCore::IncreaseSelectionListLevelCommand::doApply): Ditto.
+ * editing/SplitElementCommand.cpp:
+ (WebCore::SplitElementCommand::doApply): Ditto.
+ * editing/markup.cpp:
+ (WebCore::createFragmentFromText): Ditto.
+ * svg/SVGUseElement.cpp:
+ (WebCore::SVGUseElement::buildShadowTree): Ditto.
+ (WebCore::SVGUseElement::expandUseElementsInShadowTree): Ditto.
+
2009-03-12 Dirk Schulze <krit@webkit.org>
Reviewed by Oliver Hunt.
diff --git a/WebCore/dom/Element.cpp b/WebCore/dom/Element.cpp
index c552d73..bbd8f96 100644
--- a/WebCore/dom/Element.cpp
+++ b/WebCore/dom/Element.cpp
@@ -88,6 +88,18 @@
PassRefPtr<Node> Element::cloneNode(bool deep)
{
+ return deep ? cloneElementWithChildren() : cloneElementWithoutChildren();
+}
+
+PassRefPtr<Element> Element::cloneElementWithChildren()
+{
+ RefPtr<Element> clone = cloneElementWithoutChildren();
+ cloneChildNodes(clone.get());
+ return clone.release();
+}
+
+PassRefPtr<Element> Element::cloneElementWithoutChildren()
+{
RefPtr<Element> clone = document()->createElement(tagQName(), false);
// This will catch HTML elements in the wrong namespace that are not correctly copied.
// This is a sanity check as HTML overloads some of the DOM methods.
@@ -99,17 +111,9 @@
clone->copyNonAttributeProperties(this);
- if (deep)
- cloneChildNodes(clone.get());
-
return clone.release();
}
-PassRefPtr<Element> Element::cloneElement()
-{
- return static_pointer_cast<Element>(cloneNode(false));
-}
-
void Element::removeAttribute(const QualifiedName& name, ExceptionCode& ec)
{
if (namedAttrMap) {
diff --git a/WebCore/dom/Element.h b/WebCore/dom/Element.h
index 772371b..531802c 100644
--- a/WebCore/dom/Element.h
+++ b/WebCore/dom/Element.h
@@ -115,13 +115,13 @@
// DOM methods overridden from parent classes
virtual NodeType nodeType() const;
- virtual PassRefPtr<Node> cloneNode(bool deep);
virtual String nodeName() const;
virtual void insertedIntoDocument();
virtual void removedFromDocument();
virtual void childrenChanged(bool changedByParser = false, Node* beforeChange = 0, Node* afterChange = 0, int childCountDelta = 0);
- PassRefPtr<Element> cloneElement();
+ PassRefPtr<Element> cloneElementWithChildren();
+ PassRefPtr<Element> cloneElementWithoutChildren();
void normalizeAttributes();
@@ -219,6 +219,10 @@
virtual const AtomicString& virtualLocalName() const { return localName(); }
virtual const AtomicString& virtualNamespaceURI() const { return namespaceURI(); }
+ // cloneNode is private so that non-virtual cloneElementWithChildren and cloneElementWithoutChildren
+ // are used instead.
+ virtual PassRefPtr<Node> cloneNode(bool deep);
+
QualifiedName m_tagName;
virtual NodeRareData* createRareData();
diff --git a/WebCore/editing/ApplyStyleCommand.cpp b/WebCore/editing/ApplyStyleCommand.cpp
index d4eb247..eb31254 100644
--- a/WebCore/editing/ApplyStyleCommand.cpp
+++ b/WebCore/editing/ApplyStyleCommand.cpp
@@ -1572,7 +1572,7 @@
surroundNodeRangeWithElement(startNode, endNode, createHTMLElement(document(), supTag));
if (m_styledInlineElement)
- surroundNodeRangeWithElement(startNode, endNode, m_styledInlineElement->cloneElement());
+ surroundNodeRangeWithElement(startNode, endNode, m_styledInlineElement->cloneElementWithoutChildren());
}
float ApplyStyleCommand::computedFontSize(const Node *node)
diff --git a/WebCore/editing/BreakBlockquoteCommand.cpp b/WebCore/editing/BreakBlockquoteCommand.cpp
index 909efdd..2a513a5 100644
--- a/WebCore/editing/BreakBlockquoteCommand.cpp
+++ b/WebCore/editing/BreakBlockquoteCommand.cpp
@@ -107,7 +107,7 @@
ancestors.append(node);
// Insert a clone of the top blockquote after the break.
- RefPtr<Element> clonedBlockquote = topBlockquote->cloneElement();
+ RefPtr<Element> clonedBlockquote = topBlockquote->cloneElementWithoutChildren();
insertNodeAfter(clonedBlockquote.get(), breakNode.get());
// Clone startNode's ancestors into the cloned blockquote.
@@ -116,7 +116,7 @@
// or clonedBlockquote if ancestors is empty).
RefPtr<Element> clonedAncestor = clonedBlockquote;
for (size_t i = ancestors.size(); i != 0; --i) {
- RefPtr<Element> clonedChild = ancestors[i - 1]->cloneElement(); // shallow clone
+ RefPtr<Element> clonedChild = ancestors[i - 1]->cloneElementWithoutChildren();
// Preserve list item numbering in cloned lists.
if (clonedChild->isElementNode() && clonedChild->hasTagName(olTag)) {
Node* listChildNode = i > 1 ? ancestors[i - 2] : startNode;
diff --git a/WebCore/editing/IndentOutdentCommand.cpp b/WebCore/editing/IndentOutdentCommand.cpp
index cad160f..9444b11 100644
--- a/WebCore/editing/IndentOutdentCommand.cpp
+++ b/WebCore/editing/IndentOutdentCommand.cpp
@@ -151,7 +151,7 @@
appendNode(placeholder, listItem);
} else {
// Clone the list element, insert it before the current paragraph, and move the paragraph into it.
- RefPtr<Element> clonedList = listNode->cloneElement();
+ RefPtr<Element> clonedList = listNode->cloneElementWithoutChildren();
insertNodeBefore(clonedList, enclosingListChild(endOfCurrentParagraph.deepEquivalent().node()));
appendNode(listItem, clonedList);
appendNode(placeholder, listItem);
diff --git a/WebCore/editing/InsertParagraphSeparatorCommand.cpp b/WebCore/editing/InsertParagraphSeparatorCommand.cpp
index ec38814..adfff76 100644
--- a/WebCore/editing/InsertParagraphSeparatorCommand.cpp
+++ b/WebCore/editing/InsertParagraphSeparatorCommand.cpp
@@ -167,7 +167,7 @@
} else if (shouldUseDefaultParagraphElement(startBlock))
blockToInsert = createDefaultParagraphElement(document());
else
- blockToInsert = startBlock->cloneElement();
+ blockToInsert = startBlock->cloneElementWithoutChildren();
//---------------------------------------------------------------------
// Handle case when position is in the last visible position in its block,
@@ -274,7 +274,7 @@
// Make clones of ancestors in between the start node and the start block.
RefPtr<Element> parent = blockToInsert;
for (size_t i = ancestors.size(); i != 0; --i) {
- RefPtr<Element> child = ancestors[i - 1]->cloneElement(); // shallow clone
+ RefPtr<Element> child = ancestors[i - 1]->cloneElementWithoutChildren();
appendNode(child, parent);
parent = child.release();
}
diff --git a/WebCore/editing/ModifySelectionListLevel.cpp b/WebCore/editing/ModifySelectionListLevel.cpp
index c982989f..5ea658c 100644
--- a/WebCore/editing/ModifySelectionListLevel.cpp
+++ b/WebCore/editing/ModifySelectionListLevel.cpp
@@ -187,7 +187,7 @@
case InheritedListType:
newParent = startListChild->parentElement();
if (newParent)
- newParent = newParent->cloneElement();
+ newParent = newParent->cloneElementWithoutChildren();
break;
case OrderedList:
newParent = createOrderedListElement(document());
diff --git a/WebCore/editing/SplitElementCommand.cpp b/WebCore/editing/SplitElementCommand.cpp
index 69447d4..35dfc6f 100644
--- a/WebCore/editing/SplitElementCommand.cpp
+++ b/WebCore/editing/SplitElementCommand.cpp
@@ -43,7 +43,7 @@
void SplitElementCommand::doApply()
{
- RefPtr<Element> prefixElement = m_element2->cloneElement();
+ RefPtr<Element> prefixElement = m_element2->cloneElementWithoutChildren();
if (m_atChild->parentNode() != m_element2)
return;
diff --git a/WebCore/editing/markup.cpp b/WebCore/editing/markup.cpp
index 9c03082..1137f8d 100644
--- a/WebCore/editing/markup.cpp
+++ b/WebCore/editing/markup.cpp
@@ -1155,7 +1155,7 @@
element->setAttribute(classAttr, AppleInterchangeNewline);
} else {
if (useClonesOfEnclosingBlock)
- element = block->cloneElement();
+ element = block->cloneElementWithoutChildren();
else
element = createDefaultParagraphElement(document);
fillContainerFromString(element.get(), s);
diff --git a/WebCore/svg/SVGUseElement.cpp b/WebCore/svg/SVGUseElement.cpp
index 4eb2d9f..4ae0f5c 100644
--- a/WebCore/svg/SVGUseElement.cpp
+++ b/WebCore/svg/SVGUseElement.cpp
@@ -564,10 +564,10 @@
if (isDisallowedElement(target))
return;
- RefPtr<Node> newChild = targetInstance->correspondingElement()->cloneNode(true);
+ RefPtr<Element> newChild = targetInstance->correspondingElement()->cloneElementWithChildren();
// We don't walk the target tree element-by-element, and clone each element,
- // but instead use cloneNode(deep=true). This is an optimization for the common
+ // but instead use cloneElementWithChildren(). This is an optimization for the common
// case where <use> doesn't contain disallowed elements (ie. <foreignObject>).
// Though if there are disallowed elements in the subtree, we have to remove them.
// For instance: <use> on <g> containing <foreignObject> (indirect case).
@@ -643,10 +643,10 @@
return;
}
- RefPtr<Node> newChild = target->cloneNode(true);
+ RefPtr<Element> newChild = target->cloneElementWithChildren();
// We don't walk the target tree element-by-element, and clone each element,
- // but instead use cloneNode(deep=true). This is an optimization for the common
+ // but instead use cloneElementWithChildren(). This is an optimization for the common
// case where <use> doesn't contain disallowed elements (ie. <foreignObject>).
// Though if there are disallowed elements in the subtree, we have to remove them.
// For instance: <use> on <g> containing <foreignObject> (indirect case).
