Closed Bug 1521946 Opened 1 year ago Closed 1 year ago

Move nsSMILTypes and nsSMILValue to the mozilla namespace

Categories

(Core :: SVG, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: longsonr, Assigned: longsonr)

Details

Attachments

(1 file)

No description provided.
Attached patch 1521946.txtSplinter Review
Assignee: nobody → longsonr
Attachment #9038364 - Flags: review?(bbirtles)
Priority: -- → P3
Comment on attachment 9038364 [details] [diff] [review]
1521946.txt

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

::: dom/smil/SMILCSSValueType.cpp
@@ +16,3 @@
>  #include "nsPresContext.h"
> +#include "nsString.h"
> +#include "nsStyleUtil.h"

Thanks for fixing this ordering!

::: dom/smil/SMILCSSValueType.h
@@ +84,5 @@
>     * @param aPropID         The property that |aValue| corresponds to.
>     * @param aTargetElement  The target element to which the animation value
>     *                        applies.
>     * @param aValue          The animation value to use.
> +   * @return                A new SMILValue. On failure, returns an

Super minor nit: 'a SMILValue'.

Applies many other places in this patch but it's also probably not worth fixing unless you feel so inclined.

::: dom/svg/SVGAngle.cpp
@@ +11,1 @@
>  #include "mozilla/Move.h"

Nit: Order here

::: dom/svg/SVGAnimatedLengthList.cpp
@@ +13,2 @@
>  #include "nsSVGAttrTearoffTable.h"
> +#include "DOMSVGAnimatedLengthList.h"

I probably would have expected 'D' to come before 'n' unless you're doing a case-sensitive sort.

::: dom/svg/SVGAnimatedPathSegList.cpp
@@ +13,1 @@
>  #include "DOMSVGPathSegList.h"

Likewise here.

::: dom/svg/SVGAnimatedPreserveAspectRatio.cpp
@@ +14,1 @@
>  #include "nsSVGAttrTearoffTable.h"

Similarly here.

::: dom/svg/SVGBoolean.cpp
@@ +11,1 @@
>  #include "nsSVGAttrTearoffTable.h"

Ordering here seems odd.

::: dom/svg/SVGEnum.cpp
@@ +14,1 @@
>  #include "nsSVGAttrTearoffTable.h"

Here too (or is the idea to sort ignoring the 'ns'?).
Attachment #9038364 - Flags: review?(bbirtles) → review+

The idea is to ignore the ns because it's on my to-do list to fix that and it's just making work to keep reordering that.

I was also ignoring DOM so that DOMSVGAngle went next to SVGAngle. DOMSVGAngle is the DOM tear off for SVGAngle.

I've fixed some of the ordering, the rest can wait till I do nsSVGAttrTearoffTable

I've also had a go (via sed) at replacing an SMILValue by a SMILValue.

Pushed by longsonr@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/551abefb4933
Move nsSMILTypes and nsSMILValue to the mozilla namespace r=birtles
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.