833446 - Remove nsIDOMSVGxxxElement interfaces

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[David Zbarsky (:dzbarsky)]

I already have patches for all the classes that are on WebIDL bindings.

Comment 1

[David Zbarsky (:dzbarsky)]

Attachment: altglyph

Comment 2

[David Zbarsky (:dzbarsky)]

Attachment: animate

Comment 3

[David Zbarsky (:dzbarsky)]

Attachment: animatemotion

Comment 4

[David Zbarsky (:dzbarsky)]

Attachment: animatetransform

Comment 5

[David Zbarsky (:dzbarsky)]

Attachment: set

Comment 6

[David Zbarsky (:dzbarsky)]

Attachment: animation

Comment 7

[David Zbarsky (:dzbarsky)]

Attachment: circle

Comment 8

[David Zbarsky (:dzbarsky)]

Attachment: defs

Comment 9

[David Zbarsky (:dzbarsky)]

Attachment: desc

Comment 10

[David Zbarsky (:dzbarsky)]

Attachment: ellipse

Comment 11

[David Zbarsky (:dzbarsky)]

Attachment: g

Comment 12

[David Zbarsky (:dzbarsky)]

Attachment: gradient and subclasses

Comment 13

[David Zbarsky (:dzbarsky)]

Attachment: line

Comment 14

[David Zbarsky (:dzbarsky)]

Attachment: metadata

Comment 15

[David Zbarsky (:dzbarsky)]

Attachment: mpath

Comment 16

[David Zbarsky (:dzbarsky)]

Attachment: path

Comment 17

[David Zbarsky (:dzbarsky)]

Attachment: pattern

Comment 18

[David Zbarsky (:dzbarsky)]

Attachment: polygon

Comment 19

[David Zbarsky (:dzbarsky)]

Attachment: polyline

Comment 20

[David Zbarsky (:dzbarsky)]

Attachment: rect

Comment 21

[David Zbarsky (:dzbarsky)]

Attachment: script

Comment 22

[David Zbarsky (:dzbarsky)]

Attachment: stop

Comment 23

[David Zbarsky (:dzbarsky)]

Attachment: style

Comment 24

[David Zbarsky (:dzbarsky)]

Attachment: svg

Comment 25

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 704988 [details] [diff] [review]
pattern

This seems fine, but it might be safer to hold off on landing this until the nsJSIID::HasInstance part of bug 824589 lands.

Comment 26

[David Zbarsky (:dzbarsky)]

Attachment: foreignobject

Comment 27

[Peter Van der Beken [:peterv]]

Comment on attachment 704967 [details] [diff] [review]
altglyph

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

::: content/svg/content/src/SVGAltGlyphElement.cpp
@@ +30,5 @@
>  NS_IMPL_ADDREF_INHERITED(SVGAltGlyphElement,SVGAltGlyphElementBase)
>  NS_IMPL_RELEASE_INHERITED(SVGAltGlyphElement,SVGAltGlyphElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGAltGlyphElement)
> +  NS_NODE_INTERFACE_TABLE6(SVGAltGlyphElement, nsIDOMNode, nsIDOMElement,

Switching to the new bindings makes the NS_NODE_INTERFACE_TABLE* macros become unused, so switch to NS_IMPL_ISUPPORTS_INHERITED6.

::: dom/webidl/SVGAltGlyphElement.webidl
@@ +17,1 @@
>    attribute DOMString format;

Ugh :-(.

Comment 28

[Peter Van der Beken [:peterv]]

Comment on attachment 704968 [details] [diff] [review]
animate

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

::: content/svg/content/src/SVGAnimateElement.cpp
@@ +24,5 @@
>  NS_IMPL_ADDREF_INHERITED(SVGAnimateElement, SVGAnimationElement)
>  NS_IMPL_RELEASE_INHERITED(SVGAnimateElement, SVGAnimationElement)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGAnimateElement)
> +  NS_NODE_INTERFACE_TABLE4(SVGAnimateElement, nsIDOMNode, nsIDOMElement,

NS_IMPL_ISUPPORTS_INHERITED4

Comment 29

[Peter Van der Beken [:peterv]]

Comment on attachment 704969 [details] [diff] [review]
animatemotion

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

::: content/svg/content/src/SVGAnimateMotionElement.cpp
@@ +23,5 @@
>  NS_IMPL_ADDREF_INHERITED(SVGAnimateMotionElement, SVGAnimationElement)
>  NS_IMPL_RELEASE_INHERITED(SVGAnimateMotionElement, SVGAnimationElement)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGAnimateMotionElement)
> +  NS_NODE_INTERFACE_TABLE4(SVGAnimateMotionElement, nsIDOMNode,

NS_IMPL_ISUPPORTS_INHERITED4

Comment 30

[Peter Van der Beken [:peterv]]

Comment on attachment 704971 [details] [diff] [review]
animatetransform

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

::: content/svg/content/src/SVGAnimateTransformElement.cpp
@@ +28,2 @@
>                             nsIDOMElement, nsIDOMSVGElement,
> +                           nsIDOMSVGAnimationElement)

NS_IMPL_ISUPPORTS_INHERITED4

Comment 31

[Peter Van der Beken [:peterv]]

Comment on attachment 704973 [details] [diff] [review]
set

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

::: content/svg/content/src/SVGSetElement.cpp
@@ +24,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGSetElement, SVGAnimationElement)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGSetElement)
> +  NS_NODE_INTERFACE_TABLE4(SVGSetElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement, nsIDOMSVGAnimationElement)

NS_IMPL_ISUPPORTS_INHERITED4

Comment 32

[Peter Van der Beken [:peterv]]

Comment on attachment 704974 [details] [diff] [review]
animation

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

::: content/svg/content/src/SVGAnimateElement.cpp
@@ +25,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGAnimateElement, SVGAnimationElement)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGAnimateElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGAnimateElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

::: content/svg/content/src/SVGAnimateMotionElement.cpp
@@ +24,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGAnimateMotionElement, SVGAnimationElement)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGAnimateMotionElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGAnimateMotionElement, nsIDOMNode,
> +                           nsIDOMElement, nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

::: content/svg/content/src/SVGAnimateTransformElement.cpp
@@ +24,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGAnimateTransformElement, SVGAnimationElement)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGAnimateTransformElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGAnimateTransformElement, nsIDOMNode,
> +                           nsIDOMElement, nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

::: content/svg/content/src/SVGSetElement.cpp
@@ +24,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGSetElement, SVGAnimationElement)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGSetElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGSetElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 33

[Peter Van der Beken [:peterv]]

Comment on attachment 704975 [details] [diff] [review]
circle

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

::: content/svg/content/src/SVGCircleElement.cpp
@@ +33,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGCircleElement,SVGCircleElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGCircleElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGCircleElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 34

[Peter Van der Beken [:peterv]]

Comment on attachment 704977 [details] [diff] [review]
defs

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

::: content/svg/content/src/SVGDefsElement.cpp
@@ +24,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGDefsElement, SVGGraphicsElement)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGDefsElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGDefsElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 35

[Peter Van der Beken [:peterv]]

Comment on attachment 704978 [details] [diff] [review]
desc

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

::: content/svg/content/src/SVGDescElement.cpp
@@ +24,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGDescElement, SVGDescElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGDescElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGDescElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 36

[Peter Van der Beken [:peterv]]

Comment on attachment 704979 [details] [diff] [review]
ellipse

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

::: content/svg/content/src/SVGEllipseElement.cpp
@@ +33,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGEllipseElement,SVGEllipseElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGEllipseElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGEllipseElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

::: content/svg/content/src/SVGForeignObjectElement.cpp
@@ +35,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGForeignObjectElement, SVGGraphicsElement)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGForeignObjectElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGForeignObjectElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 37

[Peter Van der Beken [:peterv]]

Comment on attachment 704980 [details] [diff] [review]
g

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

::: content/svg/content/src/SVGGElement.cpp
@@ +24,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGGElement, SVGGraphicsElement)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGGElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGGElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 38

[Peter Van der Beken [:peterv]]

Comment on attachment 704981 [details] [diff] [review]
gradient and subclasses

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

::: content/svg/content/src/SVGGradientElement.cpp
@@ +165,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGLinearGradientElement,SVGLinearGradientElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGLinearGradientElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGLinearGradientElement, nsIDOMNode,
> +                           nsIDOMElement, nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

@@ +250,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGRadialGradientElement, SVGRadialGradientElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGRadialGradientElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGRadialGradientElement, nsIDOMNode,
> +                           nsIDOMElement, nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 39

[Peter Van der Beken [:peterv]]

Comment on attachment 704982 [details] [diff] [review]
line

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

::: content/svg/content/src/SVGLineElement.cpp
@@ +33,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGLineElement,SVGLineElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGLineElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGLineElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 40

[Peter Van der Beken [:peterv]]

Comment on attachment 704983 [details] [diff] [review]
metadata

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

::: content/svg/content/src/SVGMetadataElement.cpp
@@ +24,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGMetadataElement, SVGMetadataElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGMetadataElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGMetadataElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 41

[Peter Van der Beken [:peterv]]

Comment on attachment 704984 [details] [diff] [review]
mpath

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

::: content/svg/content/src/SVGMPathElement.cpp
@@ +51,2 @@
>                             nsIDOMSVGElement,  nsIDOMSVGURIReference,
> +                           nsIMutationObserver)

NS_IMPL_ISUPPORTS_INHERITED5

Comment 42

[Peter Van der Beken [:peterv]]

Comment on attachment 704986 [details] [diff] [review]
path

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

::: content/svg/content/src/SVGPathElement.cpp
@@ +37,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGPathElement,SVGPathElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGPathElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGPathElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 43

[Peter Van der Beken [:peterv]]

Comment on attachment 704991 [details] [diff] [review]
polygon

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

::: content/svg/content/src/SVGPolygonElement.cpp
@@ +26,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGPolygonElement,SVGPolygonElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGPolygonElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGPolygonElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 44

[Peter Van der Beken [:peterv]]

Comment on attachment 704994 [details] [diff] [review]
polyline

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

::: content/svg/content/src/SVGPolylineElement.cpp
@@ +24,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGPolylineElement,SVGPolylineElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGPolylineElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGPolylineElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 45

[Peter Van der Beken [:peterv]]

Comment on attachment 704995 [details] [diff] [review]
rect

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

::: content/svg/content/src/SVGRectElement.cpp
@@ +37,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGRectElement,SVGRectElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGRectElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGRectElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 46

[Peter Van der Beken [:peterv]]

Comment on attachment 705000 [details] [diff] [review]
script

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

::: content/svg/content/src/SVGScriptElement.cpp
@@ +38,2 @@
>                             nsIDOMSVGURIReference, nsIScriptLoaderObserver,
>                             nsIScriptElement, nsIMutationObserver)

NS_IMPL_ISUPPORTS_INHERITED7

Comment 47

[Peter Van der Beken [:peterv]]

Comment on attachment 705001 [details] [diff] [review]
stop

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

::: content/svg/content/src/SVGStopElement.cpp
@@ +27,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGStopElement, SVGStopElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGStopElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGStopElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 48

[Peter Van der Beken [:peterv]]

Comment on attachment 705003 [details] [diff] [review]
style

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

::: content/svg/content/src/SVGStyleElement.cpp
@@ +32,2 @@
>                             nsIDOMLinkStyle, nsIStyleSheetLinkingElement,
>                             nsIMutationObserver)

NS_IMPL_ISUPPORTS_INHERITED6

::: content/svg/content/src/SVGStyleElement.h
@@ +73,4 @@
>    virtual nsIDOMNode* AsDOMNode() { return this; }
>  
>    // WebIDL
> +  void GetXmlspace(nsAString & aXmlspace);

Spacing between type and & is inconsistent here :-/

Comment 49

[Peter Van der Beken [:peterv]]

Comment on attachment 705005 [details] [diff] [review]
svg

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

It doesn't seem right to ignore errors returned from SetCurrentScaleTranslate.

::: content/svg/content/src/SVGSVGElement.cpp
@@ +148,1 @@
>                             nsIDOMSVGFitToViewBox)

NS_IMPL_ISUPPORTS_INHERITED4

Comment 50

[Peter Van der Beken [:peterv]]

Comment on attachment 705200 [details] [diff] [review]
foreignobject

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

::: content/svg/content/src/SVGForeignObjectElement.cpp
@@ +36,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGForeignObjectElement, SVGGraphicsElement)
>  
>  NS_INTERFACE_TABLE_HEAD(SVGForeignObjectElement)
> +  NS_NODE_INTERFACE_TABLE3(SVGForeignObjectElement, nsIDOMNode, nsIDOMElement,
> +                           nsIDOMSVGElement)

NS_IMPL_ISUPPORTS_INHERITED3

Comment 51

[Peter Van der Beken [:peterv]]

Once this is done it looks like it might make sense to move nsIDOMSVGElement to a base class and remove all the QI/AddRef/Release overrides. >e already implement nsIDOMSVGElement mostly in a base class, right?

Comment 52

[David Zbarsky (:dzbarsky)]

(In reply to Peter Van der Beken [:peterv] from comment #51)
> Once this is done it looks like it might make sense to move nsIDOMSVGElement
> to a base class and remove all the QI/AddRef/Release overrides. >e already
> implement nsIDOMSVGElement mostly in a base class, right?

Yep, that's the plan.  Those should all move to nsSVGElement.

Comment 53

[David Zbarsky (:dzbarsky)]

(In reply to Peter Van der Beken [:peterv] from comment #49)
> Comment on attachment 705005 [details] [diff] [review]
> svg
> 
> Review of attachment 705005 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It doesn't seem right to ignore errors returned from
> SetCurrentScaleTranslate.
> 

SetCurrentScaleTranslate only fails if the arguments aren't finite, which shouldn't happen anymore.  I changed it to void.

Comment 54

[David Zbarsky (:dzbarsky)]

(In reply to Peter Van der Beken [:peterv] from comment #41)
> Comment on attachment 704984 [details] [diff] [review]
> mpath
> 
> Review of attachment 704984 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/svg/content/src/SVGMPathElement.cpp
> @@ +51,2 @@
> >                             nsIDOMSVGElement,  nsIDOMSVGURIReference,
> > +                           nsIMutationObserver)
> 
> NS_IMPL_ISUPPORTS_INHERITED5

Can't do that because of the cycle collection.

Comment 55

[David Zbarsky (:dzbarsky)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4673ca3facc
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c93f477687b
https://hg.mozilla.org/integration/mozilla-inbound/rev/e48e9622ac86
https://hg.mozilla.org/integration/mozilla-inbound/rev/c0a7d480d7c2
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c2c302ab9d2
https://hg.mozilla.org/integration/mozilla-inbound/rev/29b3563c4a03
https://hg.mozilla.org/integration/mozilla-inbound/rev/71060161c7e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/5750a71f5f78
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2fc3896a4f5
https://hg.mozilla.org/integration/mozilla-inbound/rev/208f5e11d45d
https://hg.mozilla.org/integration/mozilla-inbound/rev/e43e4368470a
https://hg.mozilla.org/integration/mozilla-inbound/rev/8005b24b6ce6
https://hg.mozilla.org/integration/mozilla-inbound/rev/194005810283
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9ced016d1e7
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf5acd1ad7c4
https://hg.mozilla.org/integration/mozilla-inbound/rev/931ee8f31546
https://hg.mozilla.org/integration/mozilla-inbound/rev/f00345f5f9ca
https://hg.mozilla.org/integration/mozilla-inbound/rev/d507fe05c49c
https://hg.mozilla.org/integration/mozilla-inbound/rev/f35d993f948a
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc3b33963618
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae929c40a462
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca55019ea1df
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d2745aa1767
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ea2ed919605
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e7305f65f73

Comment 56

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/b4673ca3facc
https://hg.mozilla.org/mozilla-central/rev/6c93f477687b
https://hg.mozilla.org/mozilla-central/rev/e48e9622ac86
https://hg.mozilla.org/mozilla-central/rev/c0a7d480d7c2
https://hg.mozilla.org/mozilla-central/rev/1c2c302ab9d2
https://hg.mozilla.org/mozilla-central/rev/29b3563c4a03
https://hg.mozilla.org/mozilla-central/rev/71060161c7e1
https://hg.mozilla.org/mozilla-central/rev/5750a71f5f78
https://hg.mozilla.org/mozilla-central/rev/c2fc3896a4f5
https://hg.mozilla.org/mozilla-central/rev/208f5e11d45d
https://hg.mozilla.org/mozilla-central/rev/e43e4368470a
https://hg.mozilla.org/mozilla-central/rev/8005b24b6ce6
https://hg.mozilla.org/mozilla-central/rev/194005810283
https://hg.mozilla.org/mozilla-central/rev/a9ced016d1e7
https://hg.mozilla.org/mozilla-central/rev/cf5acd1ad7c4
https://hg.mozilla.org/mozilla-central/rev/931ee8f31546
https://hg.mozilla.org/mozilla-central/rev/f00345f5f9ca
https://hg.mozilla.org/mozilla-central/rev/d507fe05c49c
https://hg.mozilla.org/mozilla-central/rev/f35d993f948a
https://hg.mozilla.org/mozilla-central/rev/dc3b33963618
https://hg.mozilla.org/mozilla-central/rev/ae929c40a462
https://hg.mozilla.org/mozilla-central/rev/ca55019ea1df
https://hg.mozilla.org/mozilla-central/rev/2d2745aa1767
https://hg.mozilla.org/mozilla-central/rev/8ea2ed919605
https://hg.mozilla.org/mozilla-central/rev/2e7305f65f73

Comment 57

[David Zbarsky (:dzbarsky)]

Attachment: svg v2

Comment 58

[David Zbarsky (:dzbarsky)]

Attachment: switch

Comment 59

[David Zbarsky (:dzbarsky)]

Attachment: symbol

Comment 60

[David Zbarsky (:dzbarsky)]

Attachment: text

Comment 61

[David Zbarsky (:dzbarsky)]

Attachment: use

Comment 62

[David Zbarsky (:dzbarsky)]

Attachment: view

Comment 63

[David Zbarsky (:dzbarsky)]

Attachment: clippath

Comment 64

[David Zbarsky (:dzbarsky)]

Attachment: title

Comment 65

[David Zbarsky (:dzbarsky)]

Attachment: a

Comment 66

[David Zbarsky (:dzbarsky)]

Attachment: mask

Comment 67

[David Zbarsky (:dzbarsky)]

Attachment: marker

Comment 68

[David Zbarsky (:dzbarsky)]

Attachment: image

Comment 69

[Peter Van der Beken [:peterv]]

Comment on attachment 710503 [details] [diff] [review]
svg v2

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

::: content/svg/content/src/SVGSVGElement.cpp
@@ +80,5 @@
>  
>  void
>  DOMSVGTranslatePoint::SetX(float aValue, ErrorResult& rv)
>  {
> +  mElement->SetCurrentTranslate(aValue, mPt.GetY());

Does that mean we can remove the ErrorResult for these?

@@ -722,3 @@
>  SVGSVGElement::SetCurrentScaleTranslate(float s, float x, float y)
>  {
> -  NS_ENSURE_FINITE3(s, x, y, NS_ERROR_ILLEGAL_VALUE);

Want to remove the definition of that macro too?

Comment 70

[Peter Van der Beken [:peterv]]

Comment on attachment 710509 [details] [diff] [review]
view

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

::: content/svg/content/src/SVGViewElement.cpp
@@ +49,5 @@
>  
>  SVGViewElement::SVGViewElement(already_AddRefed<nsINodeInfo> aNodeInfo)
>    : SVGViewElementBase(aNodeInfo)
>  {
> +  SetIsDOMBinding();

Ouch.

Comment 71

[Peter Van der Beken [:peterv]]

Comment on attachment 710512 [details] [diff] [review]
a

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

::: dom/interfaces/svg/nsIDOMSVGAElement.idl
@@ +12,3 @@
>  
>  [scriptable, uuid(6e0eff6e-ce35-4c01-ab3c-ae81b79b40ca)]
> +interface nsIDOMSVGAElement : nsISupports

Why are we keeping this? Addons?

Comment 72

[Peter Van der Beken [:peterv]]

Comment on attachment 710515 [details] [diff] [review]
image

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

::: content/svg/content/src/SVGImageElement.cpp
@@ +44,5 @@
>  // nsISupports methods
>  
> +NS_IMPL_ISUPPORTS_INHERITED7(SVGImageElement, SVGImageElementBase,
> +                             nsIDOMNode, nsIDOMElement,
> +                             nsIDOMSVGElement,

So I have a question about these gutted interfaces, do we not want to support QI with them anymore? And why do we still inherit from them?

Same for SVGAElement.

Comment 73

[David Zbarsky (:dzbarsky)]

(In reply to Peter Van der Beken [:peterv] from comment #69)
> Comment on attachment 710503 [details] [diff] [review]
> svg v2
> 
> Review of attachment 710503 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/svg/content/src/SVGSVGElement.cpp
> @@ +80,5 @@
> >  
> >  void
> >  DOMSVGTranslatePoint::SetX(float aValue, ErrorResult& rv)
> >  {
> > +  mElement->SetCurrentTranslate(aValue, mPt.GetY());
> 
> Does that mean we can remove the ErrorResult for these?

No, because this method is declared in nsISVGPoint and DOMSVGPoint's SetX does throw.

Comment 74

[David Zbarsky (:dzbarsky)]

(In reply to Peter Van der Beken [:peterv] from comment #71)
> Comment on attachment 710512 [details] [diff] [review]
> a
> 
> Review of attachment 710512 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/interfaces/svg/nsIDOMSVGAElement.idl
> @@ +12,3 @@
> >  
> >  [scriptable, uuid(6e0eff6e-ce35-4c01-ab3c-ae81b79b40ca)]
> > +interface nsIDOMSVGAElement : nsISupports
> 
> Why are we keeping this? Addons?

We have chrome code that does |node instanceof SVGAElement| so we need to keep this interface until you land bug 838269.

Comment 75

[David Zbarsky (:dzbarsky)]

(In reply to Peter Van der Beken [:peterv] from comment #72)
> Comment on attachment 710515 [details] [diff] [review]
> image
> 
> Review of attachment 710515 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/svg/content/src/SVGImageElement.cpp
> @@ +44,5 @@
> >  // nsISupports methods
> >  
> > +NS_IMPL_ISUPPORTS_INHERITED7(SVGImageElement, SVGImageElementBase,
> > +                             nsIDOMNode, nsIDOMElement,
> > +                             nsIDOMSVGElement,
> 
> So I have a question about these gutted interfaces, do we not want to
> support QI with them anymore? And why do we still inherit from them?
> 
> Same for SVGAElement.

Oops, I shouldn't have removed the QI for nsIDOMSVGImageElement.  So we need to keep this QI and the inheritance until cross-global instanceof.

(In reply to Peter Van der Beken [:peterv] from comment #70)
> Comment on attachment 710509 [details] [diff] [review]
> view
> 
> Review of attachment 710509 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/svg/content/src/SVGViewElement.cpp
> @@ +49,5 @@
> >  
> >  SVGViewElement::SVGViewElement(already_AddRefed<nsINodeInfo> aNodeInfo)
> >    : SVGViewElementBase(aNodeInfo)
> >  {
> > +  SetIsDOMBinding();
> 
> Ouch.

Good thing tests caught this when I changed GetClassInfo to return null.

Comment 76

[David Zbarsky (:dzbarsky)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a7581b871cc
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b925d66da97
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb13995c3194
https://hg.mozilla.org/integration/mozilla-inbound/rev/1940a3d1c6d4
https://hg.mozilla.org/integration/mozilla-inbound/rev/3386125bdee1
https://hg.mozilla.org/integration/mozilla-inbound/rev/769c0e448e1a
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e7e629ad4b9

Comment 77

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1a7581b871cc
https://hg.mozilla.org/mozilla-central/rev/5b925d66da97
https://hg.mozilla.org/mozilla-central/rev/eb13995c3194
https://hg.mozilla.org/mozilla-central/rev/1940a3d1c6d4
https://hg.mozilla.org/mozilla-central/rev/3386125bdee1
https://hg.mozilla.org/mozilla-central/rev/769c0e448e1a
https://hg.mozilla.org/mozilla-central/rev/2e7e629ad4b9

Comment 78

[David Zbarsky (:dzbarsky)]

Attachment: title updated

Comment 79

[David Zbarsky (:dzbarsky)]

Attachment: image updated

Comment 80

[David Zbarsky (:dzbarsky)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/beca55561747
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ea520af174e
https://hg.mozilla.org/integration/mozilla-inbound/rev/e20ec1200920
https://hg.mozilla.org/integration/mozilla-inbound/rev/85addbf2e264
https://hg.mozilla.org/integration/mozilla-inbound/rev/29181e9515ec

Comment 81

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/beca55561747
https://hg.mozilla.org/mozilla-central/rev/0ea520af174e
https://hg.mozilla.org/mozilla-central/rev/e20ec1200920
https://hg.mozilla.org/mozilla-central/rev/85addbf2e264
https://hg.mozilla.org/mozilla-central/rev/29181e9515ec