826947 - Convert HTMLScriptElement to WebIDL

Status: RESOLVED FIXED

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

Comment 1

[David Zbarsky (:dzbarsky)]

Attachment: Move nsHTMLScriptElement to mozilla::dom

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 698196 [details] [diff] [review]
Move nsHTMLScriptElement to mozilla::dom

>+  HTMLScriptElement(already_AddRefed<nsINodeInfo> aNodeInfo,
>                       FromParser aFromParser);

Please fix the indent.

And maybe just inline the ctor.

r=me

Comment 3

[David Zbarsky (:dzbarsky)]

Attachment: Part 2: Add WebIDL API and enable binding

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 698204 [details] [diff] [review]
Part 2: Add WebIDL API and enable binding

>+++ b/content/html/content/src/HTMLScriptElement.cpp
>+  rv = SetAttr(kNameSpaceID_None, nsGkAtoms::charset, nullptr, aCharset, true);

How about:

  SetHTMLAttr(nsGkAtoms::charset, aCharset, rv);

instead?

>+  rv = SetBoolAttr(nsGkAtoms::defer, aDefer);

  SetHTMLBoolAttr(nsGkAtoms::defer, aDefer, rv);

>+  rv = SetAttrHelper(nsGkAtoms::src, aSrc);

  SetHTMLAttr(nsGkAtoms::src, aSrc, rv);

>+  rv = SetAttr(kNameSpaceID_None, nsGkAtoms::type, nullptr, aType, true);

You get one guess for what this should look like.

>+  rv = SetAttr(kNameSpaceID_None, nsGkAtoms::_for, nullptr, aHtmlFor, true);
>+  rv = SetAttr(kNameSpaceID_None, nsGkAtoms::event, nullptr, aEvent, true);
>+  rv = SetAttr(kNameSpaceID_None, nsGkAtoms::crossorigin, nullptr, aCrossOrigin, true);

Likewise.

>+  rv = SetBoolAttr(nsGkAtoms::async, aValue);

SetHTMLBoolAttr, please.

>+++ b/dom/webidl/HTMLScriptElement.webidl
>+ * http://www.whatwg.org/specs/web-apps/current-work/multipage/obsolete.html

I think you want http://www.whatwg.org/specs/web-apps/current-work/#other-elements,-attributes-and-apis here.

>+  attribute boolean async;
>+  attribute boolean defer;

These should be [SetterThrows] too.

>+// http://www.whatwg.org/specs/web-apps/current-work/multipage/obsolete.html

See url above.

This class needs hasInstanceInterface.  There instanceof HTMLScriptElement stuff in SeaMonkey's pageinfo and all over addons.

r=me with those changes.

Comment 5

[David Zbarsky (:dzbarsky)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/8101b0c28d50
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bb0411cda329

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8101b0c28d50
https://hg.mozilla.org/mozilla-central/rev/bb0411cda329