Closed Bug 882553 Opened 7 years ago Closed 7 years ago

Convert SVGAnimatedString to WebIDL

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: dzbarsky, Assigned: dzbarsky)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
No description provided.
Attachment #761874 - Attachment is patch: true
Attachment #761874 - Attachment mime type: message/rfc822 → text/plain
Attachment #761874 - Flags: review?(Ms2ger)
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
Attachment #761874 - Flags: review?(Ms2ger) → review+
https://hg.mozilla.org/mozilla-central/rev/d8e2fa0358c4
Assignee: nobody → dzbarsky
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Depends on: 901700
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.