Closed
Bug 1415747
Opened 6 years ago
Closed 6 years ago
Remove nsIDOMHTMLScriptElement
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
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.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8927069 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 3•6 years ago
|
||
I'm a little iffy on my mForceAsync handling logic for nsIScriptElement, so please check over that carefully.
Comment 4•6 years ago
|
||
mozreview-review |
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+
Pushed by kmachulis@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8114c9ccfd4d Remove nsIDOMHTMLScriptElement; r=bz
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8114c9ccfd4d
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•