Closed Bug 826947 Opened 7 years ago Closed 7 years ago

Convert HTMLScriptElement to WebIDL

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: dzbarsky, Assigned: dzbarsky)

References

Details

Attachments

(2 files)

No description provided.
Attachment #698196 - Flags: review?(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
Attachment #698196 - Flags: review?(bzbarsky) → review+
Attachment #698204 - Flags: review?(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.
Attachment #698204 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/8101b0c28d50
https://hg.mozilla.org/mozilla-central/rev/bb0411cda329
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.