722071 - Implement array style indexing for SVGStringList

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

For compatibility with the other list types (see bug 631437), we should implement array style indexing for SVGStringList.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Mounir Lamouri (:mounir)]

Comment on attachment 592427 [details] [diff] [review]
patch

Review of attachment 592427 [details] [diff] [review]:
-----------------------------------------------------------------

Blake seems more appropriate to do this review.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 592427 [details] [diff] [review]
patch

Review of attachment 592427 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsDOMClassInfo.cpp
@@ +10912,5 @@
> +// SVGStringList helper
> +
> +NS_IMETHODIMP
> +nsSVGStringListSH::GetProperty(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
> +                               JSObject *obj, jsid id, jsval *vp, bool *_retval)

This implementation of GetProperty doesn't seem to add anything to the base class' implementation. Seems like it can be removed.

@@ +10954,5 @@
> +    list->GetLength(&length);
> +    NS_ASSERTION(PRUint32(aIndex) >= length, "Item should only return null for out-of-bounds access");
> +  }
> +#endif
> +  if (rv == NS_ERROR_DOM_INDEX_SIZE_ERR) {

Are there any tests to exercise this codepath?

Comment 4

[Jonathan Watt [:jwatt]]

(In reply to Blake Kaplan (:mrbkap) from comment #3)
> This implementation of GetProperty doesn't seem to add anything to the base
> class' implementation. Seems like it can be removed.

Ah, yeah I'll remove that.

> Are there any tests to exercise this codepath?

Yes, specifically the test that is modified in the patch (the ok() test after the 'for' loop).

Thanks for the speedy review!

Comment 5

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/mozilla-central/rev/7b0c5dcf5bcb

Comment 6

[Jeremie Patonnier :Jeremie]

Documentation is up to date :

https://developer.mozilla.org/en/DOM/SVGStringList#section_5
https://developer.mozilla.org/en/Firefox_13_for_developers