Closed Bug 1415747 Opened 2 years ago Closed 2 years ago

Remove nsIDOMHTMLScriptElement

Categories

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

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: qdot, Assigned: qdot)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Continuing post-addon-deprecation XPCOM interface cleanup.
Attachment #8927069 - Flags: review?(bzbarsky)
I'm a little iffy on my mForceAsync handling logic for nsIScriptElement, so please check over that carefully.
Comment on attachment 8927069 [details]
Bug 1415747 - Remove nsIDOMHTMLScriptElement

https://reviewboard.mozilla.org/r/198286/#review203614

::: dom/html/HTMLScriptElement.h:61
(Diff revision 2)
>                                  const nsAttrValue* aOldValue,
>                                  nsIPrincipal* aMaybeScriptedPrincipal,
>                                  bool aNotify) override;
>  
>    // WebIDL
> -  void SetText(const nsAString& aValue, ErrorResult& rv);
> +  void GetText(nsAString& aValue, ErrorResult& aRv)

Please put this out of line, especially since this header doesn't include nsContentUtils.h.

::: dom/html/HTMLScriptElement.h:68
(Diff revision 2)
> -  void SetDefer(bool aDefer, ErrorResult& rv);
> -  bool Defer();
> -  void SetSrc(const nsAString& aSrc, nsIPrincipal& aTriggeringPrincipal, ErrorResult& rv);
> -  void GetSrc(nsString& aSrc, nsIPrincipal&)
> +    if (!nsContentUtils::GetNodeTextContent(this, false, aValue, fallible)) {
> +      aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
> +    }
> +  }
> +
> +  void SetText(const nsAString& aValue, ErrorResult& aRv)

Again, out of line.

::: dom/script/nsIScriptElement.h:156
(Diff revision 2)
>      mFrozen = false;
>      mUri = nullptr;
>      mCreatorParser = nullptr;
>      mParserCreated = mozilla::dom::NOT_FROM_PARSER;
>      bool async = false;
> -    nsCOMPtr<nsIDOMHTMLScriptElement> htmlScript = do_QueryInterface(this);
> +    nsCOMPtr<nsIContent> content = do_QueryInterface(this);

Why can't we just call GetAsyncState() here without checking for an <html:script> first?  Seems like that would give the same result but be simpler...

::: dom/svg/SVGScriptElement.h:83
(Diff revision 2)
>    ~SVGScriptElement();
>  
>    virtual StringAttributesInfo GetStringInfo() override;
>  
> +  // SVG Script elements don't have the ability to set async properties on
> +  // themselves, so this will always return false.

Why not just return false explicitly?
Attachment #8927069 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/8114c9ccfd4d
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.