Improve Document exception messages.

Source/core/dom/Document.cpp

Index: Source/core/dom/Document.cpp
diff --git a/Source/core/dom/Document.cpp b/Source/core/dom/Document.cpp
index f1ba78ad20b000246d820784d84141318c5101dc..dcb5d1b7360e741877ba10fb847cae6521e87998 100644
--- a/Source/core/dom/Document.cpp
+++ b/Source/core/dom/Document.cpp
@@ -686,7 +686,7 @@ void Document::childrenChanged(bool changedByParser, Node* beforeChange, Node* a
 PassRefPtr<Element> Document::createElement(const AtomicString& name, ExceptionState& exceptionState)
 {
     if (!isValidName(name)) {
-        exceptionState.throwUninformativeAndGenericDOMException(InvalidCharacterError);
+        exceptionState.throwDOMException(InvalidCharacterError, "'" + name + "' is not a valid element name.");
         return 0;
     }
 
@@ -699,7 +699,7 @@ PassRefPtr<Element> Document::createElement(const AtomicString& name, ExceptionS
 PassRefPtr<Element> Document::createElement(const AtomicString& localName, const AtomicString& typeExtension, ExceptionState& exceptionState)
 {
     if (!isValidName(localName)) {
-        exceptionState.throwUninformativeAndGenericDOMException(InvalidCharacterError);
+        exceptionState.throwDOMException(InvalidCharacterError, "'" + localName + "' is not a valid element name.");

[sof, 2013/12/20 11:10:53]

Tangential, but are there missing checks of exceptionState for various
createElement() calls, here and below? i.e., the uses of 'element' doesn't look
safe.

[Mike West, 2013/12/20 11:38:49]

Good eye. Would you like to take care of that in a separate patch? If not,
please file a bug and I'll try to remember to take care of it. :)

[sof, 2013/12/20 12:21:12]

I can take care of it right away.

         return 0;
     }
 
@@ -724,7 +724,7 @@ PassRefPtr<Element> Document::createElementNS(const AtomicString& namespaceURI,
 
     QualifiedName qName(prefix, localName, namespaceURI);
     if (!hasValidNamespaceForElements(qName)) {
-        exceptionState.throwUninformativeAndGenericDOMException(NamespaceError);
+        exceptionState.throwDOMException(NamespaceError, "'" + namespaceURI + "' is not a valid namespace for elements.");
         return 0;
     }
 
@@ -748,7 +748,7 @@ ScriptValue Document::registerElement(WebCore::ScriptState* state, const AtomicS
 ScriptValue Document::registerElement(WebCore::ScriptState* state, const AtomicString& name, const Dictionary& options, ExceptionState& exceptionState, CustomElement::NameSet validNames)
 {
     if (!registrationContext()) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "No element registration context is available.");
         return ScriptValue();
     }
 
@@ -791,7 +791,7 @@ PassRefPtr<Comment> Document::createComment(const String& data)
 PassRefPtr<CDATASection> Document::createCDATASection(const String& data, ExceptionState& exceptionState)
 {
     if (isHTMLDocument()) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "This is an HTML document, which does not allow CData sections.");
         return 0;
     }
     if (data.contains("]]>")) {
@@ -827,7 +827,7 @@ PassRefPtr<CSSStyleDeclaration> Document::createCSSStyleDeclaration()
 PassRefPtr<Node> Document::importNode(Node* importedNode, bool deep, ExceptionState& exceptionState)
 {
     if (!importedNode) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "The node provided is invalid.");
         return 0;
     }
 
@@ -849,7 +849,7 @@ PassRefPtr<Node> Document::importNode(Node* importedNode, bool deep, ExceptionSt
         // FIXME: The following check might be unnecessary. Is it possible that
         // oldElement has mismatched prefix/namespace?
         if (!hasValidNamespaceForElements(oldElement->tagQName())) {
-            exceptionState.throwUninformativeAndGenericDOMException(NamespaceError);
+            exceptionState.throwDOMException(NamespaceError, "The imported node has an invalid namespace.");
             return 0;
         }
         RefPtr<Element> newElement = createElement(oldElement->tagQName(), false);
@@ -900,14 +900,15 @@ PassRefPtr<Node> Document::importNode(Node* importedNode, bool deep, ExceptionSt
     case XPATH_NAMESPACE_NODE:
         break;
     }
-    exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+    Element* el = toElement(importedNode);
+    exceptionState.throwDOMException(NotSupportedError, "The node to import is of type '" + el->tagName() + "', which may not be imported.");
     return 0;
 }
 
 PassRefPtr<Node> Document::adoptNode(PassRefPtr<Node> source, ExceptionState& exceptionState)
 {
     if (!source) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "The node provided is invalid.");
         return 0;
     }
 
@@ -918,9 +919,11 @@ PassRefPtr<Node> Document::adoptNode(PassRefPtr<Node> source, ExceptionState& ex
     case NOTATION_NODE:
     case DOCUMENT_NODE:
     case DOCUMENT_TYPE_NODE:
-    case XPATH_NAMESPACE_NODE:
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+    case XPATH_NAMESPACE_NODE: {
+        Element* el = toElement(source.get());
+        exceptionState.throwDOMException(NotSupportedError, "The node provided is of type '" + el->tagName() + "', which may not be adopted.");
         return 0;
+    }
     case ATTRIBUTE_NODE: {
         Attr* attr = toAttr(source.get());
         if (attr->ownerElement())
@@ -930,14 +933,14 @@ PassRefPtr<Node> Document::adoptNode(PassRefPtr<Node> source, ExceptionState& ex
     default:
         if (source->isShadowRoot()) {
             // ShadowRoot cannot disconnect itself from the host node.
-            exceptionState.throwUninformativeAndGenericDOMException(HierarchyRequestError);
+            exceptionState.throwDOMException(HierarchyRequestError, "The node provided is a shadow root, which may not be adopted.");
             return 0;
         }
 
         if (source->isFrameOwnerElement()) {
             HTMLFrameOwnerElement* frameOwnerElement = toHTMLFrameOwnerElement(source.get());
             if (frame() && frame()->tree().isDescendantOf(frameOwnerElement->contentFrame())) {
-                exceptionState.throwUninformativeAndGenericDOMException(HierarchyRequestError);
+                exceptionState.throwDOMException(HierarchyRequestError, "The node provided is a frame which contains this document.");
                 return 0;
             }
         }
@@ -1037,7 +1040,7 @@ PassRefPtr<Element> Document::createElementNS(const AtomicString& namespaceURI,
 
     QualifiedName qName(prefix, localName, namespaceURI);
     if (!hasValidNamespaceForElements(qName)) {
-        exceptionState.throwUninformativeAndGenericDOMException(NamespaceError);
+        exceptionState.throwDOMException(NamespaceError, "'" + namespaceURI + "' is an invalid namespace for elements.");
         return 0;
     }
 
@@ -1136,12 +1139,12 @@ void Document::setContentLanguage(const AtomicString& language)
 void Document::setXMLVersion(const String& version, ExceptionState& exceptionState)
 {
     if (!implementation()->hasFeature("XML", String())) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "This document does not support XML.");
         return;
     }
 
     if (!XMLDocumentParser::supportsXMLVersion(version)) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "This document does not support the XML version '" + version + "'.");
         return;
     }
 
@@ -1151,7 +1154,7 @@ void Document::setXMLVersion(const String& version, ExceptionState& exceptionSta
 void Document::setXMLStandalone(bool standalone, ExceptionState& exceptionState)
 {
     if (!implementation()->hasFeature("XML", String())) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "This document does not support XML.");
         return;
     }
 
@@ -1450,7 +1453,7 @@ PassRefPtr<NodeIterator> Document::createNodeIterator(Node* root, ExceptionState
 {
     // FIXME: Probably this should be handled within the bindings layer and TypeError should be thrown.
     if (!root) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "The provided node is invalid.");
         return 0;
     }
     return NodeIterator::create(root, NodeFilter::SHOW_ALL, PassRefPtr<NodeFilter>());
@@ -1459,7 +1462,7 @@ PassRefPtr<NodeIterator> Document::createNodeIterator(Node* root, ExceptionState
 PassRefPtr<NodeIterator> Document::createNodeIterator(Node* root, unsigned whatToShow, ExceptionState& exceptionState)
 {
     if (!root) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "The provided node is invalid.");
         return 0;
     }
     // FIXME: It might be a good idea to emit a warning if |whatToShow| contains a bit that is not defined in
@@ -1470,7 +1473,7 @@ PassRefPtr<NodeIterator> Document::createNodeIterator(Node* root, unsigned whatT
 PassRefPtr<NodeIterator> Document::createNodeIterator(Node* root, unsigned whatToShow, PassRefPtr<NodeFilter> filter, ExceptionState& exceptionState)
 {
     if (!root) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "The provided node is invalid.");
         return 0;
     }
     // FIXME: Ditto.
@@ -1480,7 +1483,7 @@ PassRefPtr<NodeIterator> Document::createNodeIterator(Node* root, unsigned whatT
 PassRefPtr<TreeWalker> Document::createTreeWalker(Node* root, ExceptionState& exceptionState)
 {
     if (!root) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "The provided node is invalid.");
         return 0;
     }
     return TreeWalker::create(root, NodeFilter::SHOW_ALL, PassRefPtr<NodeFilter>());
@@ -1489,7 +1492,7 @@ PassRefPtr<TreeWalker> Document::createTreeWalker(Node* root, ExceptionState& ex
 PassRefPtr<TreeWalker> Document::createTreeWalker(Node* root, unsigned whatToShow, ExceptionState& exceptionState)
 {
     if (!root) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "The provided node is invalid.");
         return 0;
     }
     return TreeWalker::create(root, whatToShow, PassRefPtr<NodeFilter>());
@@ -1498,7 +1501,7 @@ PassRefPtr<TreeWalker> Document::createTreeWalker(Node* root, unsigned whatToSho
 PassRefPtr<TreeWalker> Document::createTreeWalker(Node* root, unsigned whatToShow, PassRefPtr<NodeFilter> filter, ExceptionState& exceptionState)
 {
     if (!root) {
-        exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+        exceptionState.throwDOMException(NotSupportedError, "The provided node is invalid.");
         return 0;
     }
     return TreeWalker::create(root, whatToShow, filter);
@@ -2238,13 +2241,17 @@ void Document::setBody(PassRefPtr<HTMLElement> prpNewBody, ExceptionState& excep
 {
     RefPtr<HTMLElement> newBody = prpNewBody;
 
-    if (!newBody || !documentElement()) {
-        exceptionState.throwUninformativeAndGenericDOMException(HierarchyRequestError);
+    if (!newBody) {
+        exceptionState.throwDOMException(HierarchyRequestError, "The node provided is invalid.");
+        return;
+    }
+    if (!documentElement()) {
+        exceptionState.throwDOMException(HierarchyRequestError, "No document element exists.");
         return;
     }
 
     if (!newBody->hasTagName(bodyTag) && !newBody->hasTagName(framesetTag)) {
-        exceptionState.throwUninformativeAndGenericDOMException(HierarchyRequestError);
+        exceptionState.throwDOMException(HierarchyRequestError, "The new body element is of type '" + newBody->tagName() + "'. It must be either a 'BODY' or 'FRAMESET' element.");
         return;
     }
 
@@ -3657,7 +3664,7 @@ PassRefPtr<Event> Document::createEvent(const String& eventType, ExceptionState&
     if (event)
         return event.release();
 
-    exceptionState.throwUninformativeAndGenericDOMException(NotSupportedError);
+    exceptionState.throwDOMException(NotSupportedError, "The provided event type ('" + eventType + "') is invalid.");
     return 0;
 }
 
@@ -3932,7 +3939,7 @@ static bool parseQualifiedNameInternal(const AtomicString& qualifiedName, const
         U16_NEXT(characters, i, length, c)
         if (c == ':') {
             if (sawColon) {
-                exceptionState.throwUninformativeAndGenericDOMException(NamespaceError);
+                exceptionState.throwDOMException(NamespaceError, "Multiple colons (':') are not allowed.");
                 return false; // multiple colons: not allowed
             }
             nameStart = true;
@@ -3940,13 +3947,21 @@ static bool parseQualifiedNameInternal(const AtomicString& qualifiedName, const
             colonPos = i - 1;
         } else if (nameStart) {
             if (!isValidNameStart(c)) {
-                exceptionState.throwUninformativeAndGenericDOMException(InvalidCharacterError);
+                StringBuilder message;
+                message.append("The character '");
+                message.append(c);
+                message.append("' may not be used to start a name.");
+                exceptionState.throwDOMException(InvalidCharacterError, message.toString());
                 return false;
             }
             nameStart = false;
         } else {
             if (!isValidNamePart(c)) {
-                exceptionState.throwUninformativeAndGenericDOMException(InvalidCharacterError);
+                StringBuilder message;

[sof, 2013/12/20 11:10:53]

String::format() a shorter alternative?

[Mike West, 2013/12/20 11:38:49]

StringBuilder has an explicit 'append(UChar32)' method that handles random
encoding weirdness. I don't think there's an equiv. on String itself. But maybe
String::format does? I'll take a look.

+                message.append("The character '");
+                message.append(c);
+                message.append("' may not be used in a name.");
+                exceptionState.throwDOMException(InvalidCharacterError, message.toString());
                 return false;
             }
         }
@@ -3958,7 +3973,7 @@ static bool parseQualifiedNameInternal(const AtomicString& qualifiedName, const
     } else {
         prefix = AtomicString(characters, colonPos);
         if (prefix.isEmpty()) {
-            exceptionState.throwUninformativeAndGenericDOMException(NamespaceError);
+            exceptionState.throwDOMException(NamespaceError, "A colon (':') may not be used to begin a name.");
             return false;
         }
         int prefixStart = colonPos + 1;
@@ -3966,7 +3981,7 @@ static bool parseQualifiedNameInternal(const AtomicString& qualifiedName, const
     }
 
     if (localName.isEmpty()) {
-        exceptionState.throwUninformativeAndGenericDOMException(NamespaceError);
+        exceptionState.throwDOMException(NamespaceError, "The specified name is empty.");
         return false;
     }
 
@@ -3978,7 +3993,7 @@ bool Document::parseQualifiedName(const AtomicString& qualifiedName, AtomicStrin
     unsigned length = qualifiedName.length();
 
     if (!length) {
-        exceptionState.throwUninformativeAndGenericDOMException(InvalidCharacterError);
+        exceptionState.throwDOMException(InvalidCharacterError, "The provided qualified name is empty.");
         return false;
     }
 
@@ -4204,7 +4219,7 @@ PassRefPtr<Attr> Document::createAttributeNS(const AtomicString& namespaceURI, c
     QualifiedName qName(prefix, localName, namespaceURI);
 
     if (!shouldIgnoreNamespaceChecks && !hasValidNamespaceForAttributes(qName)) {
-        exceptionState.throwUninformativeAndGenericDOMException(NamespaceError);
+        exceptionState.throwDOMException(NamespaceError, "The namespace '" + namespaceURI + "' is invalid.");
         return 0;
     }