Bug 631939 - New SVG list item DOM wrappers get confused if there are more than 256 items in list. r=roc, a=roc.
authorJonathan Watt <jwatt@jwatt.org>
Tue, 08 Feb 2011 13:43:02 +0000
changeset 62137 7f6b89af70745490e1baeddac84849290c062c27
parent 62136 b25476c227531b9fd735c2beb039b77e95834e68
child 62138 5268e56c2b267c0e04712acfa741a046d4461fc1
push idunknown
push userunknown
push dateunknown
reviewersroc, roc
bugs631939
milestone2.0b12pre
Bug 631939 - New SVG list item DOM wrappers get confused if there are more than 256 items in list. r=roc, a=roc.
content/svg/content/src/DOMSVGLength.cpp
content/svg/content/src/DOMSVGLength.h
content/svg/content/src/DOMSVGNumber.cpp
content/svg/content/src/DOMSVGNumber.h
content/svg/content/src/DOMSVGPathSeg.h
content/svg/content/src/DOMSVGPoint.h
content/svg/content/test/test_SVGLengthList-2.xhtml
--- a/content/svg/content/src/DOMSVGLength.cpp
+++ b/content/svg/content/src/DOMSVGLength.cpp
@@ -73,30 +73,30 @@ namespace mozilla {
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(DOMSVGLength)
   NS_INTERFACE_MAP_ENTRY(mozilla::DOMSVGLength) // pseudo-interface
   NS_INTERFACE_MAP_ENTRY(nsIDOMSVGLength)
   NS_INTERFACE_MAP_ENTRY(nsISupports)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(SVGLength)
 NS_INTERFACE_MAP_END
 
 DOMSVGLength::DOMSVGLength(DOMSVGLengthList *aList,
-                           PRUint32 aAttrEnum,
-                           PRUint8 aListIndex,
+                           PRUint8 aAttrEnum,
+                           PRUint32 aListIndex,
                            PRUint8 aIsAnimValItem)
   : mList(aList)
   , mListIndex(aListIndex)
   , mAttrEnum(aAttrEnum)
   , mIsAnimValItem(aIsAnimValItem)
   , mUnit(nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER)
   , mValue(0.0f)
 {
   // These shifts are in sync with the members in the header.
   NS_ABORT_IF_FALSE(aList &&
-                    aAttrEnum < (1 << 22) &&
-                    aListIndex < (1 << 4) &&
+                    aAttrEnum < (1 << 4) &&
+                    aListIndex < (1 << 22) &&
                     aIsAnimValItem < (1 << 1), "bad arg");
 
   NS_ABORT_IF_FALSE(IndexIsValid(), "Bad index for DOMSVGNumber!");
 }
 
 DOMSVGLength::DOMSVGLength()
   : mList(nsnull)
   , mListIndex(0)
@@ -304,18 +304,18 @@ DOMSVGLength::ConvertToSpecifiedUnits(PR
   }
   // else [SVGWG issue] Can't convert unit
   // ReportToConsole
   return NS_ERROR_FAILURE;
 }
 
 void
 DOMSVGLength::InsertingIntoList(DOMSVGLengthList *aList,
-                                PRUint32 aAttrEnum,
-                                PRUint8 aListIndex,
+                                PRUint8 aAttrEnum,
+                                PRUint32 aListIndex,
                                 PRUint8 aIsAnimValItem)
 {
   NS_ASSERTION(!HasOwner(), "Inserting item that is already in a list");
 
   mList = aList;
   mAttrEnum = aAttrEnum;
   mListIndex = aListIndex;
   mIsAnimValItem = aIsAnimValItem;
--- a/content/svg/content/src/DOMSVGLength.h
+++ b/content/svg/content/src/DOMSVGLength.h
@@ -99,18 +99,18 @@ public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS(DOMSVGLength)
   NS_DECL_NSIDOMSVGLENGTH
 
   /**
    * Generic ctor for DOMSVGLength objects that are created for an attribute.
    */
   DOMSVGLength(DOMSVGLengthList *aList,
-               PRUint32 aAttrEnum,
-               PRUint8 aListIndex,
+               PRUint8 aAttrEnum,
+               PRUint32 aListIndex,
                PRUint8 aIsAnimValItem);
 
   /**
    * Ctor for creating the objects returned by SVGSVGElement.createSVGLength(),
    * which do not initially belong to an attribute.
    */
   DOMSVGLength();
 
@@ -152,22 +152,22 @@ public:
    * into a list, and give it the information it needs as a result.
    *
    * This object MUST NOT already belong to a list when this method is called.
    * That's not to say that script can't move these DOM objects between
    * lists - it can - it's just that the logic to handle that (and send out
    * the necessary notifications) is located elsewhere (in DOMSVGLengthList).)
    */
   void InsertingIntoList(DOMSVGLengthList *aList,
-                         PRUint32 aAttrEnum,
-                         PRUint8 aListIndex,
+                         PRUint8 aAttrEnum,
+                         PRUint32 aListIndex,
                          PRUint8 aIsAnimValItem);
 
   /// This method is called to notify this object that its list index changed.
-  void UpdateListIndex(PRUint8 aListIndex) {
+  void UpdateListIndex(PRUint32 aListIndex) {
     mListIndex = aListIndex;
   }
 
   /**
    * This method is called to notify this DOM object that it is about to be
    * removed from its current DOM list so that it can first make a copy of its
    * internal counterpart's values. (If it didn't do this, then it would
    * "lose" its value on being removed.)
--- a/content/svg/content/src/DOMSVGNumber.cpp
+++ b/content/svg/content/src/DOMSVGNumber.cpp
@@ -70,29 +70,29 @@ DOMCI_DATA(SVGNumber, DOMSVGNumber)
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(DOMSVGNumber)
   NS_INTERFACE_MAP_ENTRY(DOMSVGNumber) // pseudo-interface
   NS_INTERFACE_MAP_ENTRY(nsIDOMSVGNumber)
   NS_INTERFACE_MAP_ENTRY(nsISupports)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(SVGNumber)
 NS_INTERFACE_MAP_END
 
 DOMSVGNumber::DOMSVGNumber(DOMSVGNumberList *aList,
-                           PRUint32 aAttrEnum,
-                           PRUint8 aListIndex,
+                           PRUint8 aAttrEnum,
+                           PRUint32 aListIndex,
                            PRUint8 aIsAnimValItem)
   : mList(aList)
   , mListIndex(aListIndex)
   , mAttrEnum(aAttrEnum)
   , mIsAnimValItem(aIsAnimValItem)
   , mValue(0.0f)
 {
   // These shifts are in sync with the members in the header.
   NS_ABORT_IF_FALSE(aList &&
-                    aAttrEnum < (1 << 27) &&
-                    aListIndex < (1 << 4) &&
+                    aAttrEnum < (1 << 4) &&
+                    aListIndex < (1 << 27) &&
                     aIsAnimValItem < (1 << 1), "bad arg");
 
   NS_ABORT_IF_FALSE(IndexIsValid(), "Bad index for DOMSVGNumber!");
 }
 
 DOMSVGNumber::DOMSVGNumber()
   : mList(nsnull)
   , mListIndex(0)
@@ -134,18 +134,18 @@ DOMSVGNumber::SetValue(float aValue)
     return NS_OK;
   }
   mValue = aValue;
   return NS_OK;
 }
 
 void
 DOMSVGNumber::InsertingIntoList(DOMSVGNumberList *aList,
-                                PRUint32 aAttrEnum,
-                                PRUint8 aListIndex,
+                                PRUint8 aAttrEnum,
+                                PRUint32 aListIndex,
                                 PRUint8 aIsAnimValItem)
 {
   NS_ASSERTION(!HasOwner(), "Inserting item that is already in a list");
 
   mList = aList;
   mAttrEnum = aAttrEnum;
   mListIndex = aListIndex;
   mIsAnimValItem = aIsAnimValItem;
--- a/content/svg/content/src/DOMSVGNumber.h
+++ b/content/svg/content/src/DOMSVGNumber.h
@@ -75,18 +75,18 @@ public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS(DOMSVGNumber)
   NS_DECL_NSIDOMSVGNUMBER
 
   /**
    * Generic ctor for DOMSVGNumber objects that are created for an attribute.
    */
   DOMSVGNumber(DOMSVGNumberList *aList,
-               PRUint32 aAttrEnum,
-               PRUint8 aListIndex,
+               PRUint8 aAttrEnum,
+               PRUint32 aListIndex,
                PRUint8 aIsAnimValItem);
 
   /**
    * Ctor for creating the objects returned by SVGSVGElement.createSVGNumber(),
    * which do not initially belong to an attribute.
    */
   DOMSVGNumber();
 
@@ -125,22 +125,22 @@ public:
    * into a list, and give it the information it needs as a result.
    *
    * This object MUST NOT already belong to a list when this method is called.
    * That's not to say that script can't move these DOM objects between
    * lists - it can - it's just that the logic to handle that (and send out
    * the necessary notifications) is located elsewhere (in DOMSVGNumberList).)
    */
   void InsertingIntoList(DOMSVGNumberList *aList,
-                         PRUint32 aAttrEnum,
-                         PRUint8 aListIndex,
+                         PRUint8 aAttrEnum,
+                         PRUint32 aListIndex,
                          PRUint8 aIsAnimValItem);
 
   /// This method is called to notify this object that its list index changed.
-  void UpdateListIndex(PRUint8 aListIndex) {
+  void UpdateListIndex(PRUint32 aListIndex) {
     mListIndex = aListIndex;
   }
 
   /**
    * This method is called to notify this DOM object that it is about to be
    * removed from its current DOM list so that it can first make a copy of its
    * internal counterpart's value. (If it didn't do this, then it would
    * "lose" its value on being removed.)
--- a/content/svg/content/src/DOMSVGPathSeg.h
+++ b/content/svg/content/src/DOMSVGPathSeg.h
@@ -117,17 +117,17 @@ public:
    * lists - it can - it's just that the logic to handle that (and send out
    * the necessary notifications) is located elsewhere (in DOMSVGPathSegList).)
    */
   void InsertingIntoList(DOMSVGPathSegList *aList,
                          PRUint32 aListIndex,
                          PRBool aIsAnimValItem);
 
   /// This method is called to notify this object that its list index changed.
-  void UpdateListIndex(PRUint8 aListIndex) {
+  void UpdateListIndex(PRUint32 aListIndex) {
     mListIndex = aListIndex;
   }
 
   /**
    * This method is called to notify this DOM object that it is about to be
    * removed from its current DOM list so that it can first make a copy of its
    * internal counterpart's values. (If it didn't do this, then it would
    * "lose" its value on being removed.)
--- a/content/svg/content/src/DOMSVGPoint.h
+++ b/content/svg/content/src/DOMSVGPoint.h
@@ -171,17 +171,17 @@ public:
    * lists - it can - it's just that the logic to handle that (and send out
    * the necessary notifications) is located elsewhere (in DOMSVGPointList).)
    */
   void InsertingIntoList(DOMSVGPointList *aList,
                          PRUint32 aListIndex,
                          PRBool aIsAnimValItem);
 
   /// This method is called to notify this object that its list index changed.
-  void UpdateListIndex(PRUint8 aListIndex) {
+  void UpdateListIndex(PRUint32 aListIndex) {
     mListIndex = aListIndex;
   }
 
   /**
    * This method is called to notify this DOM object that it is about to be
    * removed from its current DOM list so that it can first make a copy of its
    * internal counterpart's values. (If it didn't do this, then it would
    * "lose" its value on being removed.)
--- a/content/svg/content/test/test_SVGLengthList-2.xhtml
+++ b/content/svg/content/test/test_SVGLengthList-2.xhtml
@@ -19,30 +19,46 @@ https://bugzilla.mozilla.org/show_bug.cg
 </svg>
 </div>
 <pre id="test">
 <script class="testbody" type="text/javascript">
 <![CDATA[
 
 SimpleTest.waitForExplicitFinish();
 
-/*
-Check that the animVal list for 'x' on <text> gives the correct number of
-items when examined for the FIRST time DURING animation.
-*/
-
 function run_tests()
 {
-  document.getElementById('svg').pauseAnimations();
+  var svg = document.getElementById('svg');
+  svg.pauseAnimations();
+
+  // Check that the animVal list for 'x' on <text> gives the correct number of
+  // items when examined for the FIRST time DURING animation:
 
   var text = document.getElementById("text");
   var list = text.x.animVal;
 
   is(list.numberOfItems, 4, 'Checking numberOfItems');
 
+  // Check that items at an index larger than 255 (max value for PRUint8) are
+  // returning the correct values:
+
+  var item;
+  list = text.x.baseVal;
+  for (var i = 0; i < 256; ++i) {
+    item = svg.createSVGLength();
+    item.value = 1;
+    list.appendItem(item);
+  }
+  item = svg.createSVGLength();
+  item.value = 2;
+  list.appendItem(item);
+
+  is(list.getItem(0).value, 1, 'Check value of first item');
+  is(list.getItem(256).value, 2, 'Check value of item at index > 255');
+
   SimpleTest.finish();
 }
 
 window.addEventListener("load", run_tests, false);
 
 ]]>
 </script>
 </pre>