Bug 970356 - neither getSubStringLength, nor selectSubString should throw if nchars is too large. r=cam
authorRobert Longson <longsonr@gmail.com>
Tue, 18 Feb 2014 14:29:27 +0000
changeset 169657 153d6e08580c4e601e5f1187c5be5a11b88daff8
parent 169656 9cb76ac4f7828537b2aedb97b7b6b194ce7c153c
child 169658 311053a3b3ade1eb8a37b9b6b0d47d994c4523e6
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewerscam
bugs970356
milestone30.0a1
Bug 970356 - neither getSubStringLength, nor selectSubString should throw if nchars is too large. r=cam
content/svg/content/test/test_getSubStringLength.xhtml
content/svg/content/test/test_selectSubString.xhtml
layout/svg/SVGTextFrame.cpp
--- a/content/svg/content/test/test_getSubStringLength.xhtml
+++ b/content/svg/content/test/test_getSubStringLength.xhtml
@@ -54,37 +54,34 @@ function runTests(text, charWidth)
     catch (e)
     {
       ok(false,
          "unexpected exception for " +
          "text.getSubStringLength(" + charnum + "," + nchars + ")");
     }
   }
 
-  expectThrow(-1, 2);
-  expectThrow(-1, 0);
-  expectThrow(1, 3);
-  expectThrow(0, 4);
+  expectThrow(100, 2);
+  expectThrow(100, 0);
   expectThrow(3, 0);
+  expectThrow(3, 1);
 
+  expectValue(1, 3, chars(2));
+  expectValue(0, 4, chars(3));
   expectValue(0, 0, chars(0));
   expectValue(1, 0, chars(0));
   expectValue(2, 0, chars(0));
   expectValue(0, 1, chars(1));
   expectValue(1, 1, chars(1));
   expectValue(2, 1, chars(1));
   expectValue(0, 2, chars(2));
   expectValue(1, 2, chars(2));
   expectValue(0, 3, chars(3));
-
-  expectThrow(1, -1);
-  expectThrow(2, -1);
-  expectThrow(3, -1);
-  expectThrow(3, -3);
-  expectThrow(-1, -1);
+  expectValue(1, 100, chars(2));
+  expectValue(2, 100, chars(1));
 }
 
 
 function run()
 {
   try
   {
     var document = $("svg").contentWindow.document;
--- a/content/svg/content/test/test_selectSubString.xhtml
+++ b/content/svg/content/test/test_selectSubString.xhtml
@@ -25,41 +25,58 @@ function runTests()
   var text = document.getElementById("text");
 
   function expectThrow(charnum, nchars)
   {
     try
     {
       text.selectSubString(charnum, nchars);
       ok(false,
-          "text.selectSubString(" + charnum + "," + nchars + ") " +
-          "should have thrown");
+         "text.selectSubString(" + charnum + "," + nchars + ") " +
+         "should have thrown");
     }
     catch (e)
     {
       is(e.name, "IndexSizeError",
-          "expected an index error for " +
-          "text.selectSubString(" + charnum + "," + nchars + ")");
+         "expected an index error for " +
+         "text.selectSubString(" + charnum + "," + nchars + ")");
       is(e.code, DOMException.INDEX_SIZE_ERR,
-          "expected an index error for " +
-          "text.selectSubString(" + charnum + "," + nchars + ")");
+         "expected an index error for " +
+         "text.selectSubString(" + charnum + "," + nchars + ")");
+    }
+  }
+
+  function expectNoThrow(charnum, nchars, expected)
+  {
+    try
+    {
+      text.selectSubString(charnum, nchars);
+      ok(true,
+         "text.selectSubString(" + charnum + "," + nchars + ") " +
+         "should not have thrown");
+    }
+    catch (e)
+    {
+      ok(false,
+         "unexpected exception for " +
+         "text.selectSubString(" + charnum + "," + nchars + ")");
     }
   }
   
-  expectThrow(-1, 2);
-  expectThrow(-1, 0);
-  expectThrow(1, 3);
-  expectThrow(0, 4);
+  expectThrow(100, 2);
+  expectThrow(100, 0);
   expectThrow(3, 0);
+  expectThrow(3, 100);
+  expectThrow(3, 100);
+  expectThrow(100, 100);
 
-  expectThrow(1, -1);
-  expectThrow(2, -1);
-  expectThrow(3, -1);
-  expectThrow(3, -3);
-  expectThrow(-1, -1);
+  expectNoThrow(1, 100);
+  expectNoThrow(2, 100);
+  expectNoThrow(1, 3);
+  expectNoThrow(0, 4);
 
   SimpleTest.finish();
 }
 
 window.addEventListener("load", runTests, false);
 </script>
 </pre>
 </body>
--- a/layout/svg/SVGTextFrame.cpp
+++ b/layout/svg/SVGTextFrame.cpp
@@ -2114,20 +2114,19 @@ public:
 
   /**
    * Advances ahead aCount matching characters.  Returns true if there were
    * enough characters to advance past, and false otherwise.
    */
   bool Next(uint32_t aCount);
 
   /**
-   * Advances ahead up to aCount matching characters, returns true if there
-   * were enough characters to advance to.
+   * Advances ahead up to aCount matching characters.
    */
-  bool NextWithinSubtree(uint32_t aCount);
+  void NextWithinSubtree(uint32_t aCount);
 
   /**
    * Advances to the character with the specified index.  The index is in the
    * space of original characters (i.e., all DOM characters under the <text>
    * that are within valid text content elements).
    */
   bool AdvanceToCharacter(uint32_t aTextElementCharIndex);
 
@@ -2416,26 +2415,25 @@ CharIterator::Next(uint32_t aCount)
     if (!Next()) {
       return false;
     }
     aCount--;
   }
   return true;
 }
 
-bool
+void
 CharIterator::NextWithinSubtree(uint32_t aCount)
 {
   while (IsWithinSubtree() && aCount) {
     --aCount;
     if (!Next()) {
-      break;
+      return;
     }
   }
-  return !aCount;
 }
 
 bool
 CharIterator::AdvanceToCharacter(uint32_t aTextElementCharIndex)
 {
   while (mTextElementCharIndex < aTextElementCharIndex) {
     if (!Next()) {
       return false;
@@ -4010,19 +4008,17 @@ SVGTextFrame::SelectSubString(nsIContent
   CharIterator chit(this, CharIterator::eAddressable, aContent);
   if (!chit.AdvanceToSubtree() ||
       !chit.Next(charnum) ||
       chit.IsAfterSubtree()) {
     return NS_ERROR_DOM_INDEX_SIZE_ERR;
   }
   charnum = chit.TextElementCharIndex();
   nsIContent* content = chit.TextFrame()->GetContent();
-  if (!chit.NextWithinSubtree(nchars)) {
-    return NS_ERROR_DOM_INDEX_SIZE_ERR;
-  }
+  chit.NextWithinSubtree(nchars);
   nchars = chit.TextElementCharIndex() - charnum;
 
   nsRefPtr<nsFrameSelection> frameSelection = GetFrameSelection();
 
   frameSelection->HandleClick(content, charnum, charnum + nchars,
                               false, false, false);
   return NS_OK;
 }
@@ -4048,19 +4044,17 @@ SVGTextFrame::GetSubStringLength(nsICont
   }
 
   if (nchars == 0) {
     *aResult = 0.0f;
     return NS_OK;
   }
 
   charnum = chit.TextElementCharIndex();
-  if (!chit.NextWithinSubtree(nchars)) {
-    return NS_ERROR_DOM_INDEX_SIZE_ERR;
-  }
+  chit.NextWithinSubtree(nchars);
   nchars = chit.TextElementCharIndex() - charnum;
 
   // Find each rendered run that intersects with the range defined
   // by charnum/nchars.
   nscoord textLength = 0;
   TextRenderedRunIterator it(this, TextRenderedRunIterator::eAllFrames);
   TextRenderedRun run = it.Current();
   while (run.mFrame) {