824898 - Convert SVG animation elements to WebIDL

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[David Zbarsky (:dzbarsky)]

Attachment: Rename files and move classes into namespaces

Comment 1

[David Zbarsky (:dzbarsky)]

Attachment: Convert to WebIDL bindings

Comment 2

[David Zbarsky (:dzbarsky)]

Attachment: Convert to WebIDL bindings

SVGAnimationElement should be concrete: false.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 695929 [details] [diff] [review]
Rename files and move classes into namespaces

Probably better to not lose the SVGWhateverElementBase typedefs on subclasses of SVGAnimationElement.  If you do want to lose them, please get review from an SVG peer on that.

>+++ b/content/svg/content/src/SVGAnimateElement.cpp
>+#include "SVGAnimateElement.h"

mozilla/dom/SVGAnimateElement.h

>+++ b/content/svg/content/src/SVGAnimateMotionElement.cpp
>+#include "SVGAnimateMotionElement.h"

mozilla/dom/SVGAnimateMotionElement.h

>+++ b/content/svg/content/src/SVGAnimateTransformElement.cpp
>+#include "SVGAnimateTransformElement.h"

mozilla/dom/SVGAnimateTransformElement.h

>+++ b/content/svg/content/src/SVGAnimationElement.cpp
>+#include "SVGAnimationElement.h"

mozilla/dom/SVGAnimationElement.h

>+++ b/content/svg/content/src/SVGSetElement.cpp
>+#include "SVGSetElement.h"

mozilla/dom/SVGSetElement.h

r=me with the above addressed

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 696817 [details] [diff] [review]
Convert to WebIDL bindings

>+++ b/content/svg/content/src/SVGAnimateElement.h
>+  virtual JSObject* WrapNode(JSContext *aCx, JSObject *aScope, bool *aTriedToWrap);

MOZ_OVERRIDE.  Same elsewhere in this patch.

>+++ b/content/svg/content/src/SVGAnimationElement.cpp
> SVGAnimationElement::GetTargetElement(nsIDOMSVGElement** aTarget)
>+  NS_IF_ADDREF(*aTarget = targetSVG);

targetSVG.forget(aTarget);

>+SVGAnimationElement::GetTargetElement()
>+  nsCOMPtr<nsSVGElement> elem = do_QueryInterface(target);

Why is this a sane thing to do?  Does nsSVGElement have an IID?

Seems like it would be better to check target->IsSVG() and if so cast to nsSVGElement*, else return null.

>+++ b/dom/bindings/Bindings.conf
>+'SVGAnimationElement': {
>+    'resultNotAddRefed': ['targetElement'],
>+    'conrete': False

'concrete'.  I wonder if we can catch typos like that somehow...

r=me with the above fixed (esp. the bogus QI bit).

Comment 5

[David Zbarsky (:dzbarsky)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7601aad6045f
https://hg.mozilla.org/integration/mozilla-inbound/rev/28cb4a10444e

Comment 6

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/7601aad6045f
https://hg.mozilla.org/mozilla-central/rev/28cb4a10444e