Created attachment 595075 [details]
###!!! ASSERTION: Item should only return null for out-of-bounds access: 'PRUint32(aIndex) >= length', file dom/base/nsDOMClassInfo.cpp, line 10924
Suspicious: 7b0c5dcf5bcb (bug 722071), 2cd5af3aca9a (bug 529328)
Created attachment 595412 [details] [diff] [review]
patch with tests
Comment on attachment 595412 [details] [diff] [review]
patch with tests
Doesn't look like this ^ is necessary. (there are no animations in the test AFAICT)
>+ ok(initializeThrowsFor(strings, null),
>+ "SVGStringList.initialize() should throw when passed null");
>+ ok(initializeThrowsFor(strings, ""),
>+ "SVGStringList.initialize() should throw when passed the empty string");
As a sanity-check for the test, it'd probably be worth adding a call to each "xxxThrowsFor" function with a *nonempty* string, like so:
"SVGStringList.initialize() shouldn't throw when passed a nonempty string");
to be sure that (a) the underlying methods don't just always throw (likely already covered elsewhere, but worth checking while you're here), and (b) your xxxThrowsFor test-methods don't just always return true due to a bug in the test.
Maybe these would belong at the end, so they don't muck up the rest of the test by modifying the string list?
Also, should this bug be marked as security-sensitive? (Comment 0 sounds scary, but the patch makes it look less scary. Not sure what "Suspicious: [pointervalue]" meant in comment 0, but those are definitely non-null values, and I'd hope we aren't somehow pulling those out of IsEmpty strings & operating on them...)
It's "Suspicious: [hg revision]", "Suspicious: [pointervalue]". There's only a correctness issue here.
Er, s$ion]", "Sus$ion]", not "Sus$
Ahhh, of course, sorry for my confusion.