Closed Bug 833446 Opened 7 years ago Closed 7 years ago

Remove nsIDOMSVGxxxElement interfaces

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: dzbarsky, Assigned: dzbarsky)

References

Details

Attachments

(36 files, 3 obsolete files)

12.29 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
8.61 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
9.31 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
9.21 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
set
8.43 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
22.82 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
10.37 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
8.74 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
8.64 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
20.89 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
g
9.47 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
26.29 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
11.04 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
8.43 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
9.30 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
28.62 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
14.31 KB, patch
bzbarsky
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
9.01 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
7.81 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
11.51 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
12.51 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
10.66 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
12.49 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
11.04 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
40.94 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
9.52 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
10.03 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
44.87 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
use
13.46 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
10.28 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
10.26 KB, patch
peterv
: review+
dzbarsky
: checkin+
Details | Diff | Splinter Review
a
11.17 KB, patch
peterv
: review+
Details | Diff | Splinter Review
12.44 KB, patch
peterv
: review+
Details | Diff | Splinter Review
25.04 KB, patch
peterv
: review+
Details | Diff | Splinter Review
8.63 KB, patch
peterv
: review+
Details | Diff | Splinter Review
12.37 KB, patch
peterv
: review+
Details | Diff | Splinter Review
I already have patches for all the classes that are on WebIDL bindings.
Attached patch altglyphSplinter Review
Attachment #704967 - Flags: review?(peterv)
Attached patch animateSplinter Review
Attachment #704968 - Flags: review?(peterv)
Attached patch animatemotionSplinter Review
Attachment #704969 - Flags: review?(peterv)
Attached patch animatetransformSplinter Review
Attachment #704971 - Flags: review?(peterv)
Attached patch setSplinter Review
Attachment #704973 - Flags: review?(peterv)
Attached patch animationSplinter Review
Attachment #704974 - Flags: review?(peterv)
Attached patch circleSplinter Review
Attachment #704975 - Flags: review?(peterv)
Attached patch defsSplinter Review
Attachment #704977 - Flags: review?(peterv)
Attached patch descSplinter Review
Attachment #704978 - Flags: review?(peterv)
Attached patch ellipseSplinter Review
Attachment #704979 - Flags: review?(peterv)
Attached patch gSplinter Review
Attachment #704980 - Flags: review?(peterv)
Attachment #704981 - Flags: review?(peterv)
Attached patch lineSplinter Review
Attachment #704982 - Flags: review?(peterv)
Attached patch metadataSplinter Review
Attachment #704983 - Flags: review?(peterv)
Attached patch mpathSplinter Review
Attachment #704984 - Flags: review?(peterv)
Attached patch pathSplinter Review
Attachment #704986 - Flags: review?(peterv)
Attached patch patternSplinter Review
Attachment #704988 - Flags: review?(bzbarsky)
Attached patch polygonSplinter Review
Attachment #704991 - Flags: review?(peterv)
Attached patch polylineSplinter Review
Attachment #704994 - Flags: review?(peterv)
Attached patch rectSplinter Review
Attachment #704995 - Flags: review?(peterv)
Attached patch scriptSplinter Review
Attachment #705000 - Flags: review?(peterv)
Attached patch stopSplinter Review
Attachment #705001 - Flags: review?(peterv)
Attached patch styleSplinter Review
Attachment #705003 - Flags: review?(peterv)
Attached patch svg (obsolete) — Splinter Review
Attachment #705005 - Flags: review?(peterv)
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.
Attachment #704988 - Flags: review?(bzbarsky) → review+
Attached patch foreignobjectSplinter Review
Attachment #705200 - Flags: review?(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 :-(.
Attachment #704967 - Flags: review?(peterv) → review+
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
Attachment #704968 - Flags: review?(peterv) → review+
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
Attachment #704969 - Flags: review?(peterv) → review+
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
Attachment #704971 - Flags: review?(peterv) → review+
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
Attachment #704973 - Flags: review?(peterv) → review+
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
Attachment #704974 - Flags: review?(peterv) → review+
Depends on: 830879
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
Attachment #704975 - Flags: review?(peterv) → review+
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
Attachment #704977 - Flags: review?(peterv) → review+
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
Attachment #704978 - Flags: review?(peterv) → review+
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
Attachment #704979 - Flags: review?(peterv) → review+
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
Attachment #704980 - Flags: review?(peterv) → review+
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
Attachment #704981 - Flags: review?(peterv) → review+
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
Attachment #704982 - Flags: review?(peterv) → review+
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
Attachment #704983 - Flags: review?(peterv) → review+
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
Attachment #704984 - Flags: review?(peterv) → review+
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
Attachment #704986 - Flags: review?(peterv) → review+
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
Attachment #704991 - Flags: review?(peterv) → review+
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
Attachment #704994 - Flags: review?(peterv) → review+
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
Attachment #704995 - Flags: review?(peterv) → review+
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
Attachment #705000 - Flags: review?(peterv) → review+
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
Attachment #705001 - Flags: review?(peterv) → review+
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 :-/
Attachment #705003 - Flags: review?(peterv) → review+
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
Attachment #705005 - Flags: review?(peterv) → review-
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
Attachment #705200 - Flags: review?(peterv) → review+
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?
(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.
(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.
(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.
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
Whiteboard: [leave open]
Attachment #704967 - Flags: checkin+
Attachment #704968 - Flags: checkin+
Attachment #704969 - Flags: checkin+
Attachment #704971 - Flags: checkin+
Attachment #704973 - Flags: checkin+
Attachment #704974 - Flags: checkin+
Attachment #704975 - Flags: checkin+
Attachment #704977 - Flags: checkin+
Attachment #704978 - Flags: checkin+
Attachment #704979 - Flags: checkin+
Attachment #704980 - Flags: checkin+
Attachment #704981 - Flags: checkin+
Attachment #704982 - Flags: checkin+
Attachment #704983 - Flags: checkin+
Attachment #704984 - Flags: checkin+
Attachment #704986 - Flags: checkin+
Attachment #704988 - Flags: checkin+
Attachment #704991 - Flags: checkin+
Attachment #704994 - Flags: checkin+
Attachment #704995 - Flags: checkin+
Attachment #705000 - Flags: checkin+
Attachment #705001 - Flags: checkin+
Attachment #705003 - Flags: checkin+
Attachment #705200 - Flags: checkin+
Attached patch svg v2Splinter Review
Attachment #710503 - Flags: review?(peterv)
Attached patch switchSplinter Review
Attachment #710505 - Flags: review?(peterv)
Attached patch symbolSplinter Review
Attachment #710506 - Flags: review?(peterv)
Attached patch textSplinter Review
Attachment #710507 - Flags: review?(peterv)
Attached patch useSplinter Review
Attachment #710508 - Flags: review?(peterv)
Attached patch viewSplinter Review
Attachment #710509 - Flags: review?(peterv)
Attached patch clippathSplinter Review
Attachment #710510 - Flags: review?(peterv)
Attached patch title (obsolete) — Splinter Review
Attached patch aSplinter Review
Attachment #710512 - Flags: review?(peterv)
Attached patch maskSplinter Review
Attachment #710513 - Flags: review?(peterv)
Attached patch markerSplinter Review
Attachment #710514 - Flags: review?(peterv)
Attached patch image (obsolete) — Splinter Review
Attachment #710515 - Flags: review?(peterv)
Attachment #710511 - Flags: review?(peterv)
Depends on: 838269
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?
Attachment #710503 - Flags: review?(peterv) → review+
Attachment #710505 - Flags: review?(peterv) → review+
Attachment #710506 - Flags: review?(peterv) → review+
Attachment #710507 - Flags: review?(peterv) → review+
Attachment #710508 - Flags: review?(peterv) → review+
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.
Attachment #710509 - Flags: review?(peterv) → review+
Attachment #710510 - Flags: review?(peterv) → review+
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?
Attachment #710512 - Flags: review?(peterv) → review+
Attachment #710513 - Flags: review?(peterv) → review+
Attachment #710514 - Flags: review?(peterv) → review+
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.
(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.
(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.
(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.
Attachment #710503 - Flags: checkin+
Attachment #710505 - Flags: checkin+
Attachment #710506 - Flags: checkin+
Attachment #710507 - Flags: checkin+
Attachment #710508 - Flags: checkin+
Attachment #710509 - Flags: checkin+
Attachment #710510 - Flags: checkin+
Attachment #705005 - Attachment is obsolete: true
Attached patch title updatedSplinter Review
Attachment #710511 - Attachment is obsolete: true
Attachment #710511 - Flags: review?(peterv)
Attachment #714982 - Flags: review?(peterv)
Attached patch image updatedSplinter Review
Attachment #710515 - Attachment is obsolete: true
Attachment #710515 - Flags: review?(peterv)
Attachment #714983 - Flags: review?(peterv)
Attachment #714982 - Flags: review?(peterv) → review+
Attachment #714983 - Flags: review?(peterv) → review+
You need to log in before you can comment on or make changes to this bug.