The code implementing these classes has diverged. A base class would rectify
this and prevent it from happening again, not to mention reduce bloat.
SVGStringList (not implemented)
Created attachment 250685 [details] [diff] [review]
First draft patch of common base class(es) with a rewrite of nsSVGNumberList
This is my first pass at a base SVG list class - it compiles but I'm not totally satisfied with the design and I need some review - basically I'm not happy with all the casting that goes on in the DOM methods inside nsSVGNumberList.cpp now.
My original idea was to make the base list class a templated subclass, but I think this would grow the size of the executable, wouldn't it? Let me know if you think this is a better idea.
Woops, nsSVGListBase::AppendElement() needs to call EnsureElementIsParentless(aElement) just like nsSVGListBase::InsertElementAt().
Also, I notice that the DOM method Initialize (see http://www.w3.org/TR/SVG11/paths.html#InterfaceSVGPathSegList) does not explicitly state that the item inserted must be removed from its previous list, but I'm guessing that's the intent. I'll ask for clarity from the SVG WG.
jwatt has mentioned that it might be a good idea to wrap up the declaration and definition of a derived list class into a C-macro for that one-line creation goodness (to avoid the code divergence that happened).
Also, would it be possible to do this in a cleaner way if I introduced a new interface class nsISVGListItem ? I've never done that before so I avoided it in my first patch, but maybe I'll attempt something once I get some feedback here.
Note to self. Must make sure the base class takes into account Bug 369249. See http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=&subdir=mozilla/content/svg/content/src&command=DIFF_FRAMESET&root=&file=nsSVGLengthList.cpp&rev1=1.11&rev2=1.12
(In reply to comment #4)
> Also, would it be possible to do this in a cleaner way if I introduced a new
> interface class nsISVGListItem ? I've never done that before so I avoided it
> in my first patch, but maybe I'll attempt something once I get some feedback
I don't think you want an interface class. We're getting rid of those where possible. You just end up with lots of virtual methods and CallQueryInterface calls which are slow.
Note. Must make sure the base class takes into account Bug 435209.
The list code was rewritten in bug 515116.