Closed Bug 1522650 Opened 8 months ago Closed 8 months ago

Complete migration of SMIL to the mozilla namespace

Categories

(Core :: SVG, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: longsonr, Assigned: longsonr)

Details

Attachments

(1 file)

No description provided.
Attached patch patchSplinter Review
Assignee: nobody → longsonr
Attachment #9038914 - Flags: review?(bbirtles)
Comment on attachment 9038914 [details] [diff] [review]
patch

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

Thanks for doing this. It looks great (thanks for taking care of the comments, dropping typedefs etc.).

Regarding the updated include guards, feel free to do that in a separate patch/bug if you prefer.

::: dom/base/Element.h
@@ +18,5 @@
> +#include "nsAttrValueInlines.h"
> +#include "nsChangeHint.h"
> +#include "nsContentUtils.h"
> +#include "nsDOMAttributeMap.h"
> +#include "DOMIntersectionObserver.h"

(I'm a little surprised by the "sort by ignoring the 'ns' prefix" approach, but if that's what you and dholbert discussed, or that's what you're seeing elsewhere, it's fine with me.)

::: dom/smil/SMILAttr.h
@@ +100,1 @@
>  #endif  // NS_ISMILATTR_H_

I forgot to check this in the other patches in this series, but I think we need to update these include guards to use:

  mozilla_SMILAttr_h

etc.

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#CC_practices

::: dom/svg/SVGAnimationElement.h
@@ -21,5 @@
> -enum nsSMILTargetAttrType {
> -  eSMILTargetAttrType_auto,
> -  eSMILTargetAttrType_CSS,
> -  eSMILTargetAttrType_XML
> -};

Nice catch!
Attachment #9038914 - Flags: review?(bbirtles) → review+
Pushed by longsonr@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c985b12ab62
Rename all remaining nsSMIL classes and types as SMIL and ensure they are in the mozilla namespace. r=birtles

I left the include headers for another bug, I did make the ordering in Element.h look less odd though.

Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.