Bug 1451672 - part 17: Rename EditorBase::InsertContainerAbove() to EditorBase::InsertContainerWithTransactionInternal() and wraps it with new inline methods, EditorBase::InsertContainerWithTransaction() r?m_kato
Similar to EditorBase::ReplaceContainerWithTransaction(),
EditorBase::InsertContainerAbove() may set an attribute to newly created element
if it's specified. However, for avoiding the null check, let's make them
as references rather than pointers and treat nsGkAtoms::_empty as nullptr for
making the code safer.
This patch removes "Above" from the method name since it's redundant.
"Insert" sounds like inserting a node, and "Container" means to keep existing
children with new element in EditorBase.
MozReview-Commit-ID: 6EnkKHynYSP
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -1773,68 +1773,66 @@ EditorBase::RemoveContainerWithTransacti
nsresult rv = DeleteNodeWithTransaction(aElement);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
return NS_OK;
}
-/**
- * InsertContainerAbove() inserts a new parent for inNode, which is contructed
- * to be of type aNodeType. outNode becomes a child of inNode's earlier
- * parent. Caller's responsibility to make sure inNode's can be child of
- * outNode, and outNode can be child of old parent.
- */
already_AddRefed<Element>
-EditorBase::InsertContainerAbove(nsIContent* aNode,
- nsAtom* aNodeType,
- nsAtom* aAttribute,
- const nsAString* aValue)
-{
- MOZ_ASSERT(aNode && aNodeType);
-
- EditorDOMPoint pointToInsertNewContainer(aNode);
+EditorBase::InsertContainerWithTransactionInternal(
+ nsIContent& aContent,
+ nsAtom& aTagName,
+ nsAtom& aAttribute,
+ const nsAString& aAttributeValue)
+{
+ EditorDOMPoint pointToInsertNewContainer(&aContent);
if (NS_WARN_IF(!pointToInsertNewContainer.IsSet())) {
return nullptr;
}
- // aNode will be moved to the new container before inserting the new
+ // aContent will be moved to the new container before inserting the new
// container. So, when we insert the container, the insertion point
- // is before the next sibling of aNode.
+ // is before the next sibling of aContent.
+ // XXX If pointerToInsertNewContainer stores offset here, the offset and
+ // referring child node become mismatched. Although, currently this
+ // is not a problem since InsertNodeTransaction refers only child node.
DebugOnly<bool> advanced = pointToInsertNewContainer.AdvanceOffset();
NS_WARNING_ASSERTION(advanced,
- "Failed to advance offset to after aNode");
-
- // Create new container
- RefPtr<Element> newContainer = CreateHTMLContent(aNodeType);
+ "Failed to advance offset to after aContent");
+
+ // Create new container.
+ RefPtr<Element> newContainer = CreateHTMLContent(&aTagName);
if (NS_WARN_IF(!newContainer)) {
return nullptr;
}
- // Set attribute if needed
- if (aAttribute && aValue && aAttribute != nsGkAtoms::_empty) {
+ // Set attribute if needed.
+ if (&aAttribute != nsGkAtoms::_empty) {
nsresult rv =
- newContainer->SetAttr(kNameSpaceID_None, aAttribute, *aValue, true);
+ newContainer->SetAttr(kNameSpaceID_None, &aAttribute, aAttributeValue,
+ true);
if (NS_WARN_IF(NS_FAILED(rv))) {
return nullptr;
}
}
// Notify our internal selection state listener
AutoInsertContainerSelNotify selNotify(mRangeUpdater);
// Put aNode in the new container, first.
- nsresult rv = DeleteNodeWithTransaction(*aNode);
+ nsresult rv = DeleteNodeWithTransaction(aContent);
if (NS_WARN_IF(NS_FAILED(rv))) {
return nullptr;
}
{
AutoTransactionsConserveSelection conserveSelection(this);
- rv = InsertNodeWithTransaction(*aNode, EditorRawDOMPoint(newContainer, 0));
+ rv = InsertNodeWithTransaction(aContent,
+ EditorRawDOMPoint(newContainer, 0));
if (NS_WARN_IF(NS_FAILED(rv))) {
return nullptr;
}
}
// Put the new container where aNode was.
rv = InsertNodeWithTransaction(*newContainer, pointToInsertNewContainer);
if (NS_WARN_IF(NS_FAILED(rv))) {
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -430,21 +430,63 @@ public:
/**
* RemoveContainerWithTransaction() removes aElement from the DOM tree and
* moves all its children to the parent of aElement.
*
* @param aElement The element to be removed.
*/
nsresult RemoveContainerWithTransaction(Element& aElement);
- already_AddRefed<Element> InsertContainerAbove(nsIContent* aNode,
- nsAtom* aNodeType,
- nsAtom* aAttribute = nullptr,
- const nsAString* aValue =
- nullptr);
+ /**
+ * InsertContainerWithTransaction() creates new element whose name is
+ * aTagName, moves aContent into the new element, then, inserts the new
+ * element into where aContent was.
+ * Note that this method does not check if aContent is valid child of
+ * the new element. So, callers need to guarantee it.
+ *
+ * @param aContent The content which will be wrapped with new
+ * element.
+ * @param aTagName Element name of new element which will wrap
+ * aContent and be inserted into where aContent
+ * was.
+ * @return The new element.
+ */
+ already_AddRefed<Element>
+ InsertContainerWithTransaction(nsIContent& aContent, nsAtom& aTagName)
+ {
+ return InsertContainerWithTransactionInternal(aContent, aTagName,
+ *nsGkAtoms::_empty,
+ EmptyString());
+ }
+
+ /**
+ * InsertContainerWithTransaction() creates new element whose name is
+ * aTagName, sets its aAttribute to aAttributeValue, moves aContent into the
+ * new element, then, inserts the new element into where aContent was.
+ * Note that this method does not check if aContent is valid child of
+ * the new element. So, callers need to guarantee it.
+ *
+ * @param aContent The content which will be wrapped with new
+ * element.
+ * @param aTagName Element name of new element which will wrap
+ * aContent and be inserted into where aContent
+ * was.
+ * @param aAttribute Attribute which should be set to the new
+ * element.
+ * @param aAttributeValue Value to be set to aAttribute.
+ * @return The new element.
+ */
+ already_AddRefed<Element>
+ InsertContainerWithTransaction(nsIContent& aContent, nsAtom& aTagName,
+ nsAtom& aAttribute,
+ const nsAString& aAttributeValue)
+ {
+ return InsertContainerWithTransactionInternal(aContent, aTagName,
+ aAttribute, aAttributeValue);
+ }
/**
* SplitNodeWithTransaction() creates a transaction to create a new node
* (left node) identical to an existing node (right node), and split the
* contents between the same point in both nodes, then, execute the
* transaction.
*
* @param aStartOfRightNode The point to split. Its container will be
@@ -732,16 +774,40 @@ protected:
already_AddRefed<Element>
ReplaceContainerWithTransactionInternal(Element& aElement,
nsAtom& aTagName,
nsAtom& aAttribute,
const nsAString& aAttributeValue,
bool aCloneAllAttributes);
/**
+ * InsertContainerWithTransactionInternal() creates new element whose name is
+ * aTagName, moves aContent into the new element, then, inserts the new
+ * element into where aContent was. If aAttribute is not nsGkAtoms::_empty,
+ * aAttribute of the new element will be set to aAttributeValue.
+ *
+ * @param aContent The content which will be wrapped with new
+ * element.
+ * @param aTagName Element name of new element which will wrap
+ * aContent and be inserted into where aContent
+ * was.
+ * @param aAttribute Attribute which should be set to the new
+ * element. If this is nsGkAtoms::_empty,
+ * this does not set any attributes to the new
+ * element.
+ * @param aAttributeValue Value to be set to aAttribute.
+ * @return The new element.
+ */
+ already_AddRefed<Element>
+ InsertContainerWithTransactionInternal(nsIContent& aContent,
+ nsAtom& aTagName,
+ nsAtom& aAttribute,
+ const nsAString& aAttributeValue);
+
+ /**
* Called after a transaction is done successfully.
*/
void DoAfterDoTransaction(nsITransaction *aTxn);
/**
* Called after a transaction is undone successfully.
*/
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -3897,18 +3897,21 @@ HTMLEditRules::WillMakeList(Selection* a
if (curNode->IsHTMLElement(nsGkAtoms::p)) {
listItem =
htmlEditor->ReplaceContainerWithTransaction(*curNode->AsElement(),
*itemType);
if (NS_WARN_IF(!listItem)) {
return NS_ERROR_FAILURE;
}
} else {
- listItem = htmlEditor->InsertContainerAbove(curNode, itemType);
- NS_ENSURE_STATE(listItem);
+ listItem =
+ htmlEditor->InsertContainerWithTransaction(*curNode, *itemType);
+ if (NS_WARN_IF(!listItem)) {
+ return NS_ERROR_FAILURE;
+ }
}
if (IsInlineNode(curNode)) {
prevListItem = listItem;
} else {
prevListItem = nullptr;
}
}
} else {
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -4548,19 +4548,22 @@ HTMLEditor::CopyLastEditableChildStyles(
if (childNode) {
childElement = childNode->IsElement() ? childNode->AsElement()
: childNode->GetParentElement();
}
while (childElement && (childElement != aPreviousBlock)) {
if (HTMLEditUtils::IsInlineStyle(childElement) ||
childElement->IsHTMLElement(nsGkAtoms::span)) {
if (newStyles) {
- newStyles = InsertContainerAbove(newStyles,
- childElement->NodeInfo()->NameAtom());
- NS_ENSURE_STATE(newStyles);
+ newStyles =
+ InsertContainerWithTransaction(*newStyles,
+ *childElement->NodeInfo()->NameAtom());
+ if (NS_WARN_IF(!newStyles)) {
+ return NS_ERROR_FAILURE;
+ }
} else {
EditorRawDOMPoint atStartOfNewBlock(newBlock, 0);
deepestStyle = newStyles =
CreateNodeWithTransaction(*childElement->NodeInfo()->NameAtom(),
atStartOfNewBlock);
if (NS_WARN_IF(!newStyles)) {
return NS_ERROR_FAILURE;
}
--- a/editor/libeditor/HTMLStyleEditor.cpp
+++ b/editor/libeditor/HTMLStyleEditor.cpp
@@ -398,25 +398,27 @@ HTMLEditor::SetInlinePropertyOnNodeImpl(
bool useCSS = (IsCSSEnabled() &&
CSSEditUtils::IsCSSEditableProperty(&aNode, &aProperty,
aAttribute)) ||
// bgcolor is always done using CSS
aAttribute == nsGkAtoms::bgcolor;
if (useCSS) {
- nsCOMPtr<dom::Element> tmp;
+ RefPtr<dom::Element> tmp;
// We only add style="" to <span>s with no attributes (bug 746515). If we
// don't have one, we need to make one.
if (aNode.IsHTMLElement(nsGkAtoms::span) &&
!aNode.AsElement()->GetAttrCount()) {
tmp = aNode.AsElement();
} else {
- tmp = InsertContainerAbove(&aNode, nsGkAtoms::span);
- NS_ENSURE_STATE(tmp);
+ tmp = InsertContainerWithTransaction(aNode, *nsGkAtoms::span);
+ if (NS_WARN_IF(!tmp)) {
+ return NS_ERROR_FAILURE;
+ }
}
// Add the CSS styles corresponding to the HTML style request
mCSSEditUtils->SetCSSEquivalentToHTMLStyle(tmp,
&aProperty, aAttribute,
&aValue, false);
return NS_OK;
}
@@ -426,20 +428,24 @@ HTMLEditor::SetInlinePropertyOnNodeImpl(
if (NS_WARN_IF(!aAttribute)) {
return NS_ERROR_FAILURE;
}
// Just set the attribute on it.
return SetAttributeWithTransaction(*aNode.AsElement(), *aAttribute, aValue);
}
// ok, chuck it in its very own container
- RefPtr<Element> tmp = InsertContainerAbove(&aNode, &aProperty, aAttribute,
- &aValue);
- NS_ENSURE_STATE(tmp);
-
+ RefPtr<Element> tmp =
+ InsertContainerWithTransaction(aNode, aProperty,
+ aAttribute ? *aAttribute :
+ *nsGkAtoms::_empty,
+ aValue);
+ if (NS_WARN_IF(!tmp)) {
+ return NS_ERROR_FAILURE;
+ }
return NS_OK;
}
nsresult
HTMLEditor::SetInlinePropertyOnNode(nsIContent& aNode,
nsAtom& aProperty,
nsAtom* aAttribute,
const nsAString& aValue)
@@ -730,17 +736,17 @@ HTMLEditor::RemoveStyleInside(nsIContent
bool hasClassAttr =
aNode.AsElement()->HasAttr(kNameSpaceID_None, nsGkAtoms::_class);
if (aProperty && (hasStyleAttr || hasClassAttr)) {
// aNode carries inline styles or a class attribute so we can't
// just remove the element... We need to create above the element
// a span that will carry those styles or class, then we can delete
// the node.
RefPtr<Element> spanNode =
- InsertContainerAbove(&aNode, nsGkAtoms::span);
+ InsertContainerWithTransaction(aNode, *nsGkAtoms::span);
if (NS_WARN_IF(!spanNode)) {
return NS_ERROR_FAILURE;
}
nsresult rv =
CloneAttributeWithTransaction(*nsGkAtoms::style, *spanNode,
*aNode.AsElement());
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
@@ -1524,20 +1530,21 @@ HTMLEditor::RelativeFontChangeOnTextNode
EditorRawDOMPoint(sibling, 0));
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
return NS_OK;
}
// Else reparent the node inside font node with appropriate relative size
- nsCOMPtr<Element> newElement =
- InsertContainerAbove(textNodeForTheRange, nodeType);
- NS_ENSURE_STATE(newElement);
-
+ RefPtr<Element> newElement =
+ InsertContainerWithTransaction(*textNodeForTheRange, *nodeType);
+ if (NS_WARN_IF(!newElement)) {
+ return NS_ERROR_FAILURE;
+ }
return NS_OK;
}
nsresult
HTMLEditor::RelativeFontChangeHelper(int32_t aSizeChange,
nsINode* aNode)
{
MOZ_ASSERT(aNode);
@@ -1632,19 +1639,20 @@ HTMLEditor::RelativeFontChangeOnNode(int
sibling = GetNextHTMLSibling(aNode);
if (sibling && sibling->IsHTMLElement(atom)) {
// following sib is already right kind of inline node; slide this over into it
return MoveNodeWithTransaction(*aNode, EditorRawDOMPoint(sibling, 0));
}
// else insert it above aNode
- nsCOMPtr<Element> newElement = InsertContainerAbove(aNode, atom);
- NS_ENSURE_STATE(newElement);
-
+ RefPtr<Element> newElement = InsertContainerWithTransaction(*aNode, *atom);
+ if (NS_WARN_IF(!newElement)) {
+ return NS_ERROR_FAILURE;
+ }
return NS_OK;
}
// none of the above? then cycle through the children.
// MOOSE: we should group the children together if possible
// into a single "big" or "small". For the moment they are
// each getting their own.
AutoTArray<nsCOMPtr<nsIContent>, 10> childList;