Closed Bug 1518666 Opened 10 months ago Closed 8 months ago

Do some refactoring in support of moving nsSVGLength2 and nsSVGNumber2 into the mozilla namespace

Categories

(Core :: SVG, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: longsonr, Assigned: longsonr)

Details

Attachments

(5 files)

No description provided.

I propose

a) move SVGLength to SVGLengthListItem
b) move nsSVGLength2 to SVGLength

Same for SVGNumber/nsSVGNumber2

I'm aware of bug 591808 but I'm not sure that

a) fits with the naming of all other things e.g. SVGAngle, DOMSVGAngle and SVGAnimatedAngle
b) what would happen to the existing SVGAnimatedLength class

suggestions appreciated before I start running sed to rename everything

Flags: needinfo?(jwatt)
Priority: -- → P3

I think I'd rather rename SVGAnimatedLength to DOMSVGAnimatedLength, then rename nsSVGLength2 to SVGAnimatedLength (and leave SVGLength as it is). That nsSVGLength2 is actually two values (a base and anim value) should be reflected in its name I think. Does that sound ok?

Flags: needinfo?(jwatt)
Keywords: leave-open
Assignee: nobody → longsonr
Pushed by longsonr@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f19a83763e50
part 1 - Remove a couple of unused include files r=dholbert

nsSVGLength2 should be called SVGAnimatedLength, however there is already a SVGAnimatedLength class.
We'll rename SVGAnimatedLength (and similar classes) to DOMSVGAnimatedLength in preparation.

(In reply to Robert Longson [:longsonr] from comment #5)

We'll rename SVGAnimatedLength (and similar classes) to DOMSVGAnimatedLength in preparation.

So just to be sure I understand: is the idea here that...

  • our internal representations will be named SVGFoo
  • the dom wrapper versions will all be named DOMSVGFoo
    ?

I think that makes sense, just want to be sure there aren't important subtleties/exceptions that I'm missing.

Flags: needinfo?(longsonr)

Yes, that's it.

Flags: needinfo?(longsonr)
Pushed by longsonr@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a76a71448ff
part 2 - Move existing SVGAnimated classes to DOMSVGAnimated so that we can rename various existing classes to SVGAnimated later r=dholbert

SVGViewBox is going to become SVGAnimatedViewBox its simpler to avoid using it in this spot

We're going to rename some of these classes and we don't need the typedefs so remove them

Note: you might consider morphing this bug to be titled e.g. "Do some refactoring in support of moving nsSVGLength2 and nsSVGNumber2 into the mozilla namespace", and file a new bug for the still-to-be-done tasks here.

(It looks like this bug already spans 2 releases -- part 1 landed in Firefox 67 Nightly, and subsequent parts will be landing in various weeks of Firefox 68 Nightly, I think. If some part here happens to cause a regression that we find out about in a month or so, then it becomes nontrivial to e.g. figure out which releases might be affected, what needs backing out & where, etc. in a bug like this whose patches span multiple weeks/releases.)

Pushed by longsonr@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a808ee3490a
Part 3 - Avoid instantiating an SVGViewBox pointer r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/b18ce0338351
Part 4 - remove some unnecessary typedefs r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9ff0f5790a0
Part 5 - SVGAnimatedPathSegList does not need to be exported r=dholbert
Summary: Move nsSVGLength2 and nsSVGNumber2 into the mozilla namespace → Do some refactoring in support of moving nsSVGLength2 and nsSVGNumber2 into the mozilla namespace
Status: NEW → RESOLVED
Closed: 8 months ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.