Improve 'Selection' exception messages.

Source/core/page/DOMSelection.cpp

 /*
  * Copyright (C) 2007, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 181 matching lines @@
         return 0;
     return m_frame->selection()->isNone() ? 0 : 1;
 }
 
 void DOMSelection::collapse(Node* node, int offset, ExceptionState& es)
 {
     if (!m_frame)
         return;
 
     if (offset < 0) {
-        es.throwDOMException(IndexSizeError);
+        es.throwDOMException(IndexSizeError, "Failed to execute 'collapse' on a 'Selection' object: " + String::number(offset) + " is not a valid offset.");
         return;
     }
 
     if (!isValidForPosition(node))
         return;
 
     // FIXME: Eliminate legacy editing positions
     m_frame->selection()->moveTo(VisiblePosition(createLegacyEditingPosition(node, offset), DOWNSTREAM));
 }
 
 void DOMSelection::collapseToEnd(ExceptionState& es)
 {
     if (!m_frame)
         return;
 
     const VisibleSelection& selection = m_frame->selection()->selection();
 
     if (selection.isNone()) {
-        es.throwDOMException(InvalidStateError);
+        es.throwDOMException(InvalidStateError, "Failed to execute 'collapseToEnd' on a 'Selection' object: there is no selection.");

[arv (Not doing code reviews), 2013/08/13 13:57:31]

I feel like we are repeating ourselves a lot. Maybe use String::format?

es.throwDOMException(InvalidStateError, String::format(failedToExecutePattern,
"collapseToEnd", "Selection", "there is no selection"));

where pattern is defined somewhere where it can be reused

failedToExecutePattern = "Failed to execute '%s' on a '%s' object: %s."

[Mike West, 2013/08/13 14:00:17]

I can do that. I have vague performance reluctance to use String::format, but
given the rarity with which exceptions should be thrown, I guess it's nothing to
really worry about.

[arv (Not doing code reviews), 2013/08/13 14:07:49]

Yeah, I don't think we need to optimize for the exception case ;-)

         return;
     }
 
     m_frame->selection()->moveTo(VisiblePosition(selection.end(), DOWNSTREAM));
 }
 
 void DOMSelection::collapseToStart(ExceptionState& es)
 {
     if (!m_frame)
         return;
 
     const VisibleSelection& selection = m_frame->selection()->selection();
 
     if (selection.isNone()) {
-        es.throwDOMException(InvalidStateError);
+        es.throwDOMException(InvalidStateError, "Failed to execute 'collapseToStart' on a 'Selection' object: there is no selection.");
         return;
     }
 
     m_frame->selection()->moveTo(VisiblePosition(selection.start(), DOWNSTREAM));
 }
 
 void DOMSelection::empty()
 {
     if (!m_frame)
         return;
     m_frame->selection()->clear();
 }
 
 void DOMSelection::setBaseAndExtent(Node* baseNode, int baseOffset, Node* extentNode, int extentOffset, ExceptionState& es)
 {
     if (!m_frame)
         return;
 
-    if (baseOffset < 0 || extentOffset < 0) {
-        es.throwDOMException(IndexSizeError);
+    if (baseOffset < 0) {
+        es.throwDOMException(IndexSizeError, "Failed to execute 'setBaseAndExtent' on a 'Selection' object: " + String::number(baseOffset) + " is not a valid base offset.");
         return;
     }
 
+    if (extentOffset < 0) {
+        es.throwDOMException(IndexSizeError, "Failed to execute 'setBaseAndExtent' on a 'Selection' object: " + String::number(extentOffset) + " is not a valid extent offset.");
+        return;
+    }
+
     if (!isValidForPosition(baseNode) || !isValidForPosition(extentNode))
         return;
 
     // FIXME: Eliminate legacy editing positions
     VisiblePosition visibleBase = VisiblePosition(createLegacyEditingPosition(baseNode, baseOffset), DOWNSTREAM);
     VisiblePosition visibleExtent = VisiblePosition(createLegacyEditingPosition(extentNode, extentOffset), DOWNSTREAM);
 
     m_frame->selection()->moveTo(visibleBase, visibleExtent);
 }
 
 void DOMSelection::setPosition(Node* node, int offset, ExceptionState& es)
 {
     if (!m_frame)
         return;
     if (offset < 0) {
-        es.throwDOMException(IndexSizeError);
+        es.throwDOMException(IndexSizeError, "Failed to execute 'setPosition' on a 'Selection' object: " + String::number(offset) + " is not a valid offset.");
         return;
     }
 
     if (!isValidForPosition(node))
         return;
 
     // FIXME: Eliminate legacy editing positions
     m_frame->selection()->moveTo(VisiblePosition(createLegacyEditingPosition(node, offset), DOWNSTREAM));
 }
 
@@ skipping 46 matching lines @@
 
     m_frame->selection()->modify(alter, direction, granularity);
 }
 
 void DOMSelection::extend(Node* node, int offset, ExceptionState& es)
 {
     if (!m_frame)
         return;
 
     if (!node) {
-        es.throwDOMException(TypeMismatchError);
+        es.throwDOMException(TypeMismatchError, "Failed to execute 'extend' on a 'Selection' object: The node provided is invalid.");
         return;
     }
 
-    if (offset < 0 || offset > (node->offsetInCharacters() ? caretMaxOffset(node) : (int)node->childNodeCount())) {
-        es.throwDOMException(IndexSizeError);
+    if (offset < 0) {
+        es.throwDOMException(IndexSizeError, "Failed to execute 'extend' on a 'Selection' object: " + String::number(offset) + " is not a valid offset.");
+        return;
+    }
+    if (offset > (node->offsetInCharacters() ? caretMaxOffset(node) : (int)node->childNodeCount())) {
+        es.throwDOMException(IndexSizeError, "Failed to execute 'extend' on a 'Selection' object: " + String::number(offset) + " is larger than the given node's length.");
         return;
     }
 
     if (!isValidForPosition(node))
         return;
 
     // FIXME: Eliminate legacy editing positions
     m_frame->selection()->setExtent(VisiblePosition(createLegacyEditingPosition(node, offset), DOWNSTREAM));
 }
 
 PassRefPtr<Range> DOMSelection::getRangeAt(int index, ExceptionState& es)
 {
     if (!m_frame)
         return 0;
 
     if (index < 0 || index >= rangeCount()) {
-        es.throwDOMException(IndexSizeError);
+        es.throwDOMException(IndexSizeError, "Failed to execute 'getRangeAt' on a 'Selection' object: " + String::number(index) + " is not a valid index.");
         return 0;
     }
 
     // If you're hitting this, you've added broken multi-range selection support
     ASSERT(rangeCount() == 1);
 
     if (Node* shadowAncestor = selectionShadowAncestor(m_frame)) {
         ContainerNode* container = shadowAncestor->parentNodeGuaranteedHostFree();
         int offset = shadowAncestor->nodeIndex();
         return Range::create(shadowAncestor->document(), container, offset, container, offset);
@@ skipping 157 matching lines @@
 
 bool DOMSelection::isValidForPosition(Node* node) const
 {
     ASSERT(m_frame);
     if (!node)
         return true;
     return node->document() == m_frame->document();
 }
 
 } // namespace WebCore