1415747 - Remove nsIDOMHTMLScriptElement

Status: RESOLVED FIXED

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

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Continuing post-addon-deprecation XPCOM interface cleanup.

Comment 1

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Bug 1415747 - Remove nsIDOMHTMLScriptElement

Review commit: https://reviewboard.mozilla.org/r/198286/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/198286/

Comment 2

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8927069 [details]
Bug 1415747 - Remove nsIDOMHTMLScriptElement

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198286/diff/1-2/

Comment 3

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

I'm a little iffy on my mForceAsync handling logic for nsIScriptElement, so please check over that carefully.

Comment 4

[Boris Zbarsky [:bzbarsky]]

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?

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8114c9ccfd4d
Remove nsIDOMHTMLScriptElement; r=bz

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/8114c9ccfd4d