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
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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>