882553 - Convert SVGAnimatedString to WebIDL

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[David Zbarsky (:dzbarsky)]

Attachment: Patch

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 761874 [details] [diff] [review]
Patch

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

Nice.

::: content/svg/content/src/SVGAnimatedString.h
@@ +17,5 @@
> +{
> +public:
> +  SVGAnimatedString(nsSVGElement* aSVGElement)
> +    : mSVGElement(aSVGElement)
> +  { SetIsDOMBinding(); }

Three lines for the body, please

@@ +21,5 @@
> +  { SetIsDOMBinding(); }
> +
> +  virtual JSObject* WrapObject(JSContext* aCx,
> +                               JS::Handle<JSObject*> aScope) MOZ_OVERRIDE
> +  { return SVGAnimatedStringBinding::Wrap(aCx, aScope, this); }

Please move this out of line and don't include SVGAnimatedStringBinding.h in the header

@@ +24,5 @@
> +                               JS::Handle<JSObject*> aScope) MOZ_OVERRIDE
> +  { return SVGAnimatedStringBinding::Wrap(aCx, aScope, this); }
> +
> +  // WebIDL
> +  nsSVGElement* GetParentObject() { return mSVGElement; }

Four lines, and const

::: content/svg/content/src/nsSVGClass.cpp
@@ +19,2 @@
>  
> +  DOMAnimatedString(nsSVGClass *aVal, nsSVGElement *aSVGElement)

* to the left

@@ +19,3 @@
>  
> +  DOMAnimatedString(nsSVGClass *aVal, nsSVGElement *aSVGElement)
> +    : mozilla::dom::SVGAnimatedString(aSVGElement), mVal(aVal) {}

Format as:

  DOMAnimatedString(nsSVGClass* aVal, nsSVGElement* aSVGElement)
    : SVGAnimatedString(aSVGElement)
    , mVal(aVal)
  {}

@@ +24,3 @@
>  
> +  void GetBaseVal(nsAString& aResult) MOZ_OVERRIDE
> +    { mVal->GetBaseValue(aResult, mSVGElement); }

And this as

  void GetBaseVal(nsAString& aResult) MOZ_OVERRIDE
  {
    mVal->GetBaseValue(aResult, mSVGElement);
  }

@@ +24,5 @@
>  
> +  void GetBaseVal(nsAString& aResult) MOZ_OVERRIDE
> +    { mVal->GetBaseValue(aResult, mSVGElement); }
> +  void SetBaseVal(const nsAString& aValue) MOZ_OVERRIDE
> +    { mVal->SetBaseValue(aValue, mSVGElement, true); }

Ditto

::: content/svg/content/src/nsSVGString.h
@@ +58,3 @@
>  
>      DOMAnimatedString(nsSVGString *aVal, nsSVGElement *aSVGElement)
> +      : mozilla::dom::SVGAnimatedString(aSVGElement), mVal(aVal) {}

Indentation as before

@@ +64,5 @@
>  
> +    void GetBaseVal(nsAString & aResult) MOZ_OVERRIDE
> +      { mVal->GetBaseValue(aResult, mSVGElement); }
> +    void SetBaseVal(const nsAString & aValue) MOZ_OVERRIDE
> +      { mVal->SetBaseValue(aValue, mSVGElement, true); }

Ditto

Comment 2

[David Zbarsky (:dzbarsky)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d8e2fa0358c4

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d8e2fa0358c4