1516727 - Having classes called SVGAngle and nsSVGAngle is confusing

Status: RESOLVED FIXED

(Core :: SVG, enhancement, P3)

Comment 1

[Robert Longson [:longsonr]]

Attachment: Part 1 - simplify creation of SVGAngle objects

Comment 2

[Jonathan Watt [:jwatt]]

Comment on attachment 9033598 [details] [diff] [review]
Part 1 - simplify creation of SVGAngle objects

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

::: dom/svg/SVGAngle.cpp
@@ +19,5 @@
> +SVGAngle::SVGAngle(SVGElement* aSVGElement)
> +    : mSVGElement(aSVGElement), mType(SVGAngle::CreatedValue) {
> +  nsSVGAngle* angle = new nsSVGAngle();
> +  angle->Init();
> +  mVal = angle;

Given that we don't check for a failure status from Init() you could just assign directly to mVal here.

::: dom/svg/SVGSVGElement.cpp
@@ +266,1 @@
>    return svgangle.forget();

You could use:

  return do_AddRef(new SVGAngle(this));

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f534d2edbd06
Part 1 simplify creation of SVGAngle objects r=jwatt

Comment 4

[Robert Longson [:longsonr]]

Attachment: Part 2 - Rename SVGAngle to DOMSVGAngle

Comment 5

[Cosmin Sabou [:CosminS]]

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

Comment 6

[Jonathan Watt [:jwatt]]

Comment on attachment 9033682 [details] [diff] [review]
Part 2 - Rename SVGAngle to DOMSVGAngle

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

Can you run `./mach clang-format` before requesting review for future patches? (I'm assuming you're all set up to be able to run that.) Not a big deal, but it's nice to see the patch as it will land.

::: dom/svg/SVGSVGElement.cpp
@@ +14,5 @@
>  #include "mozilla/EventDispatcher.h"
>  #include "mozilla/SMILAnimationController.h"
>  
> +#include "DOMSVGAngle.h"
> +#include "DOMSVGAngle.h"

No need to repeat this. ;)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/43b1e9034e34
Part 2 rename SVGAngle to DOMSVGAngle r=jwatt

Comment 8

[Robert Longson [:longsonr]]

Attachment: Part 3 - rename nsSVGAngle to SVGAngle

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/43b1e9034e34

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e43ba4cbf93
Part 3 rename nsSVGAngle to SVGAngle r=jwatt

Comment 11

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/5e43ba4cbf93

Comment 12

[Robert Longson [:longsonr]]

Attachment: Part 4 - move nsSVGAngle to the mozilla namespace

I mistakenly thought it was already in the mozilla namespace like nsSVGTransform, but apparently not.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9196c1cefbb
Part 4 move SVGAngle to the mozilla namespace r=jwatt

Comment 14

[Narcis Beleuzu [:NarcisB]]

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