Improve 'ContainerNode' exception messages.

Source/core/dom/ContainerNode.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * Library General Public License for more details.
  *
  * You should have received a copy of the GNU Library General Public License
  * along with this library; see the file COPYING.LIB.  If not, write to
  * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
  * Boston, MA 02110-1301, USA.
  */
 
 #include "config.h"
 #include "core/dom/ContainerNode.h"
 
+#include "bindings/v8/ExceptionMessages.h"
 #include "bindings/v8/ExceptionState.h"
 #include "bindings/v8/ExceptionStatePlaceholder.h"
 #include "core/dom/ChildListMutationScope.h"
 #include "core/dom/ContainerNodeAlgorithms.h"
 #include "core/dom/ElementTraversal.h"
 #include "core/events/EventNames.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/FullscreenElementStack.h"
 #include "core/events/MutationEvent.h"
 #include "core/dom/NodeRareData.h"
@@ skipping 97 matching lines @@
     return &document == document.templateDocument();
 }
 
 static inline bool containsConsideringHostElements(const Node* newChild, const Node* newParent)
 {
     return (newParent->isInShadowTree() || isInTemplateContent(newParent))
         ? newChild->containsIncludingHostElements(newParent)
         : newChild->contains(newParent);
 }
 
-static inline bool checkAcceptChild(ContainerNode* newParent, Node* newChild, Node* oldChild, ExceptionState& es)
+static inline bool checkAcceptChild(ContainerNode* newParent, Node* newChild, Node* oldChild, const String& method, ExceptionState& es)
 {
     // Not mentioned in spec: throw NotFoundError if newChild is null
     if (!newChild) {
-        es.throwUninformativeAndGenericDOMException(NotFoundError);
+        es.throwDOMException(NotFoundError, ExceptionMessages::failedToExecute(method, "ContainerNode", "The new child element is null."));

[arv (Not doing code reviews), 2013/09/26 15:23:10]

Let's not use ContainerNode in any user visible messages. It is an
implementation detail. It should just be Node.

         return false;
     }
 
     // Use common case fast path if possible.
     if ((newChild->isElementNode() || newChild->isTextNode()) && newParent->isElementNode()) {
         ASSERT(!newParent->isDocumentTypeNode());
         ASSERT(isChildTypeAllowed(newParent, newChild));
         if (containsConsideringHostElements(newChild, newParent)) {
-            es.throwUninformativeAndGenericDOMException(HierarchyRequestError);
+            es.throwDOMException(HierarchyRequestError, ExceptionMessages::failedToExecute(method, "ContainerNode", "The new child element contains the parent."));
             return false;
         }
         return true;
     }
 
     // This should never happen, but also protect release builds from tree corruption.
     ASSERT(!newChild->isPseudoElement());
     if (newChild->isPseudoElement()) {
-        es.throwUninformativeAndGenericDOMException(HierarchyRequestError);
+        es.throwDOMException(HierarchyRequestError, ExceptionMessages::failedToExecute(method, "ContainerNode", "The new child element is a pseudo-element."));
         return false;
     }
 
     if (containsConsideringHostElements(newChild, newParent)) {
-        es.throwUninformativeAndGenericDOMException(HierarchyRequestError);
+        es.throwDOMException(HierarchyRequestError, ExceptionMessages::failedToExecute(method, "ContainerNode", "The new child element contains the parent."));
         return false;
     }
 
     if (oldChild && newParent->isDocumentNode()) {
         if (!toDocument(newParent)->canReplaceChild(newChild, oldChild)) {
-            es.throwUninformativeAndGenericDOMException(HierarchyRequestError);
+            // FIXME: Adjust 'Document::canReplaceChild' to return some additional detail (or an error message).
+            es.throwDOMException(HierarchyRequestError, ExceptionMessages::failedToExecute(method, "ContainerNode"));
             return false;
         }
     } else if (!isChildTypeAllowed(newParent, newChild)) {
-        es.throwUninformativeAndGenericDOMException(HierarchyRequestError);
+        es.throwDOMException(HierarchyRequestError, ExceptionMessages::failedToExecute(method, "ContainerNode", "Nodes of type '" + newChild->nodeName() + "' may not be inserted inside nodes of type '" + newParent->nodeName() + "'."));
         return false;
     }
 
     return true;
 }
 
-static inline bool checkAcceptChildGuaranteedNodeTypes(ContainerNode* newParent, Node* newChild, ExceptionState& es)
+static inline bool checkAcceptChildGuaranteedNodeTypes(ContainerNode* newParent, Node* newChild, const String& method, ExceptionState& es)
 {
     ASSERT(!newParent->isDocumentTypeNode());
     ASSERT(isChildTypeAllowed(newParent, newChild));
     if (newChild->contains(newParent)) {
-        es.throwUninformativeAndGenericDOMException(HierarchyRequestError);
+        es.throwDOMException(HierarchyRequestError, ExceptionMessages::failedToExecute(method, "ContainerNode", "The new child element contains the parent."));
         return false;
     }
 
     return true;
 }
 
-static inline bool checkAddChild(ContainerNode* newParent, Node* newChild, ExceptionState& es)
+static inline bool checkAddChild(ContainerNode* newParent, Node* newChild, const String& method, ExceptionState& es)
 {
-    return checkAcceptChild(newParent, newChild, 0, es);
+    return checkAcceptChild(newParent, newChild, 0, method, es);
 }
 
-static inline bool checkReplaceChild(ContainerNode* newParent, Node* newChild, Node* oldChild, ExceptionState& es)
+static inline bool checkReplaceChild(ContainerNode* newParent, Node* newChild, Node* oldChild, const String& method, ExceptionState& es)
 {
-    return checkAcceptChild(newParent, newChild, oldChild, es);
+    return checkAcceptChild(newParent, newChild, oldChild, method, es);
 }
 
 void ContainerNode::insertBefore(PassRefPtr<Node> newChild, Node* refChild, ExceptionState& es)
 {
     // Check that this node is not "floating".
     // If it is, it can be deleted as a side effect of sending mutation events.
     ASSERT(refCount() || parentOrShadowHostNode());
 
     RefPtr<Node> protect(this);
 
     // insertBefore(node, 0) is equivalent to appendChild(node)
     if (!refChild) {
         appendChild(newChild, es);
         return;
     }
 
     // Make sure adding the new child is OK.
-    if (!checkAddChild(this, newChild.get(), es))
+    if (!checkAddChild(this, newChild.get(), "insertBefore", es))
         return;
 
     // NotFoundError: Raised if refChild is not a child of this node
     if (refChild->parentNode() != this) {
-        es.throwUninformativeAndGenericDOMException(NotFoundError);
+        es.throwDOMException(NotFoundError, ExceptionMessages::failedToExecute("insertBefore", "ContainerNode", "The node before which the new node is to be inserted is not a child of this node."));
         return;
     }
 
     if (refChild->previousSibling() == newChild || refChild == newChild) // nothing to do
         return;
 
     RefPtr<Node> next = refChild;
 
     NodeVector targets;
     collectChildrenAndRemoveFromOldParent(newChild.get(), targets, es);
     if (es.hadException())
         return;
     if (targets.isEmpty())
         return;
 
     // We need this extra check because collectChildrenAndRemoveFromOldParent() can fire mutation events.
-    if (!checkAcceptChildGuaranteedNodeTypes(this, newChild.get(), es))
+    if (!checkAcceptChildGuaranteedNodeTypes(this, newChild.get(), "insertBefore", es))
         return;
 
     InspectorInstrumentation::willInsertDOMNode(&document(), this);
 
     ChildListMutationScope mutation(this);
     for (NodeVector::const_iterator it = targets.begin(); it != targets.end(); ++it) {
         Node* child = it->get();
 
         // Due to arbitrary code running in response to a DOM mutation event it's
         // possible that "next" is no longer a child of "this".
@@ skipping 70 matching lines @@
     // Check that this node is not "floating".
     // If it is, it can be deleted as a side effect of sending mutation events.
     ASSERT(refCount() || parentOrShadowHostNode());
 
     RefPtr<Node> protect(this);
 
     if (oldChild == newChild) // nothing to do
         return;
 
     if (!oldChild) {
-        es.throwUninformativeAndGenericDOMException(NotFoundError);
+        es.throwDOMException(NotFoundError, ExceptionMessages::failedToExecute("replaceChild", "ContainerNode", "The node to be replaced is null."));
         return;
     }
 
     // Make sure replacing the old child with the new is ok
-    if (!checkReplaceChild(this, newChild.get(), oldChild, es))
+    if (!checkReplaceChild(this, newChild.get(), oldChild, "replaceChild", es))
         return;
 
     // NotFoundError: Raised if oldChild is not a child of this node.
     if (oldChild->parentNode() != this) {
-        es.throwUninformativeAndGenericDOMException(NotFoundError);
+        es.throwDOMException(NotFoundError, ExceptionMessages::failedToExecute("replaceChild", "ContainerNode", "The node to be replaced is not a child of this node."));
         return;
     }
 
     ChildListMutationScope mutation(this);
 
     RefPtr<Node> next = oldChild->nextSibling();
 
     // Remove the node we're replacing
     RefPtr<Node> removedChild = oldChild;
     removeChild(oldChild, es);
     if (es.hadException())
         return;
 
     if (next && (next->previousSibling() == newChild || next == newChild)) // nothing to do
         return;
 
     // Does this one more time because removeChild() fires a MutationEvent.
-    if (!checkReplaceChild(this, newChild.get(), oldChild, es))
+    if (!checkReplaceChild(this, newChild.get(), oldChild, "replaceChild", es))
         return;
 
     NodeVector targets;
     collectChildrenAndRemoveFromOldParent(newChild.get(), targets, es);
     if (es.hadException())
         return;
 
     // Does this yet another check because collectChildrenAndRemoveFromOldParent() fires a MutationEvent.
-    if (!checkReplaceChild(this, newChild.get(), oldChild, es))
+    if (!checkReplaceChild(this, newChild.get(), oldChild, "replaceChild", es))
         return;
 
     InspectorInstrumentation::willInsertDOMNode(&document(), this);
 
     // Add the new child(ren)
     for (NodeVector::const_iterator it = targets.begin(); it != targets.end(); ++it) {
         Node* child = it->get();
 
         // Due to arbitrary code running in response to a DOM mutation event it's
         // possible that "next" is no longer a child of "this".
@@ skipping 59 matching lines @@
 void ContainerNode::removeChild(Node* oldChild, ExceptionState& es)
 {
     // Check that this node is not "floating".
     // If it is, it can be deleted as a side effect of sending mutation events.
     ASSERT(refCount() || parentOrShadowHostNode());
 
     RefPtr<Node> protect(this);
 
     // NotFoundError: Raised if oldChild is not a child of this node.
     if (!oldChild || oldChild->parentNode() != this) {
-        es.throwUninformativeAndGenericDOMException(NotFoundError);
+        es.throwDOMException(NotFoundError, ExceptionMessages::failedToExecute("removeChild", "ContainerNode", "The node to be removed is not a child of this node."));
         return;
     }
 
     RefPtr<Node> child = oldChild;
 
     document().removeFocusedElementOfSubtree(child.get());
 
     if (FullscreenElementStack* fullscreen = FullscreenElementStack::fromIfExists(&document()))
         fullscreen->removeFullScreenElementOfSubtree(child.get());
 
     // Events fired when blurring currently focused node might have moved this
     // child into a different parent.
     if (child->parentNode() != this) {
-        es.throwUninformativeAndGenericDOMException(NotFoundError);
+        es.throwDOMException(NotFoundError, ExceptionMessages::failedToExecute("removeChild", "ContainerNode", "The node to be removed is no longer a child of this node. Perhaps it was moved in a 'blur' event handler?"));
         return;
     }
 
     willRemoveChild(child.get());
 
     // Mutation events might have moved this child into a different parent.
     if (child->parentNode() != this) {
-        es.throwUninformativeAndGenericDOMException(NotFoundError);
+        es.throwDOMException(NotFoundError, ExceptionMessages::failedToExecute("removeChild", "ContainerNode", "The node to be removed is no longer a child of this node. Perhaps it was moved in response to a mutation?"));
         return;
     }
 
     {
         WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
 
         Node* prev = child->previousSibling();
         Node* next = child->nextSibling();
         removeBetween(prev, next, child.get());
         childrenChanged(false, prev, next, -1);
@@ skipping 101 matching lines @@
 
 void ContainerNode::appendChild(PassRefPtr<Node> newChild, ExceptionState& es)
 {
     RefPtr<ContainerNode> protect(this);
 
     // Check that this node is not "floating".
     // If it is, it can be deleted as a side effect of sending mutation events.
     ASSERT(refCount() || parentOrShadowHostNode());
 
     // Make sure adding the new child is ok
-    if (!checkAddChild(this, newChild.get(), es))
+    if (!checkAddChild(this, newChild.get(), "appendChild", es))
         return;
 
     if (newChild == m_lastChild) // nothing to do
         return;
 
     NodeVector targets;
     collectChildrenAndRemoveFromOldParent(newChild.get(), targets, es);
     if (es.hadException())
         return;
 
     if (targets.isEmpty())
         return;
 
     // We need this extra check because collectChildrenAndRemoveFromOldParent() can fire mutation events.
-    if (!checkAcceptChildGuaranteedNodeTypes(this, newChild.get(), es))
+    if (!checkAcceptChildGuaranteedNodeTypes(this, newChild.get(), "appendChild", es))
         return;
 
     InspectorInstrumentation::willInsertDOMNode(&document(), this);
 
     // Now actually add the child(ren)
     ChildListMutationScope mutation(this);
     for (NodeVector::const_iterator it = targets.begin(); it != targets.end(); ++it) {
         Node* child = it->get();
 
         // If the child has a parent again, just stop what we're doing, because
@@ skipping 375 matching lines @@
         return true;
 
     if (node->isElementNode() && toElement(node)->shadow())
         return true;
 
     return false;
 }
 #endif
 
 } // namespace WebCore