Closed
Bug 722071
Opened 13 years ago
Closed 13 years ago
Implement array style indexing for SVGStringList
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: jwatt, Assigned: jwatt)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file)
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
For compatibility with the other list types (see bug 631437), we should implement array style indexing for SVGStringList.
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #592427 -
Flags: review?(mounir)
Comment 2•13 years ago
|
||
Comment on attachment 592427 [details] [diff] [review]
patch
Review of attachment 592427 [details] [diff] [review]:
-----------------------------------------------------------------
Blake seems more appropriate to do this review.
Attachment #592427 -
Flags: review?(mounir) → review?(mrbkap)
Comment 3•13 years ago
|
||
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?
Attachment #592427 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 4•13 years ago
|
||
(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!
Updated•13 years ago
|
Target Milestone: --- → mozilla13
Assignee | ||
Comment 5•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Keywords: dev-doc-needed
Comment 6•13 years ago
|
||
Documentation is up to date :
https://developer.mozilla.org/en/DOM/SVGStringList#section_5
https://developer.mozilla.org/en/Firefox_13_for_developers
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•