Closed Bug 827119 Opened 7 years ago Closed 7 years ago

Convert SVGSVGElement to WebIDL

Categories

(Core :: SVG, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: dzbarsky, Assigned: dzbarsky)

References

Details

Attachments

(2 files)

No description provided.
Attachment #698431 - Flags: review?(bzbarsky)
Attachment #698432 - Flags: review?(bzbarsky)
Blocks: 827200
Comment on attachment 698431 [details] [diff] [review]
move SVGSVGElement to mozilla::dom

>-  NS_INTERFACE_MAP_ENTRY(nsISVGPoint)
>+  NS_INTERFACE_MAP_ENTRY(mozilla::nsISVGPoint)

Why is this change needed?  This is all inside mozilla::dom... is there a mozilla::dom::nsISVGPoint that you're trying to avoid?

r=me with that fixed or explained.
Attachment #698431 - Flags: review?(bzbarsky) → review+
Comment on attachment 698432 [details] [diff] [review]
Convert SVGSVGElement to WebIDL

You should probably comment that the XPCOM version of SetCurrentScale and SetCurrentTranslate are OK for us because they only throw on non-finite floats and we don't allow those anyway.

Same thing for unsuspendRedraw(All).

>+bool
>+SVGSVGElement::CheckIntersection(nsSVGElement& element,
..
>+  return nullptr;

This compiled?  return false, please.  Same for CheckEnclosure and DeselectAll.

> SVGSVGElement::CreateSVGLength(nsIDOMSVGLength **_retval)
> {
>+  *_retval = CreateSVGLength().get();
>   NS_ADDREF(*_retval = new DOMSVGLength());

Doesn't that leak?  Nuke the NS_ADDREF line?

r=me with those fixed.
Attachment #698432 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky (:bz) from comment #3)
> Comment on attachment 698431 [details] [diff] [review]
> move SVGSVGElement to mozilla::dom
> 
> >-  NS_INTERFACE_MAP_ENTRY(nsISVGPoint)
> >+  NS_INTERFACE_MAP_ENTRY(mozilla::nsISVGPoint)
> 
> Why is this change needed?  This is all inside mozilla::dom... is there a
> mozilla::dom::nsISVGPoint that you're trying to avoid?
> 
> r=me with that fixed or explained.

This is needed because NS_INTERFACE_MAP_ENTRY calls NS_GET_IID, which expands as
#define NS_GET_IID(T) (::T::COMTypeInfo<int>::kIID)
(In reply to Boris Zbarsky (:bz) from comment #4)
> Comment on attachment 698432 [details] [diff] [review]
> Convert SVGSVGElement to WebIDL
> 
> >+bool
> >+SVGSVGElement::CheckIntersection(nsSVGElement& element,
> ..
> >+  return nullptr;
> 
> This compiled?  return false, please.  Same for CheckEnclosure and
> DeselectAll.

That's strange.  I'm removing these in another patch anyway so it doesn't matter.
> (::T::COMTypeInfo<int>::kIID)

Hmm.  So how did it use to work?  I guess "using" puts things in the global namespace?

In any case, add a comment please.

> I'm removing these in another patch anyway so it doesn't matter.

I fully expect it to not build on all compilers, so just fix before you land.
You need to log in before you can comment on or make changes to this bug.