Closed Bug 1444394 Opened 3 years ago Closed 2 years ago

[tracking] Remove Element::UnsafeSetInnerHTML

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: johannh, Assigned: johannh)

References

Details

(Keywords: meta)

Attachments

(1 file)

Bug 1432966 introduced the unsafeSetInnerHTML function to avoid dealing with all the cases where our sanitizer would have broken innerHTML functionality.

We should track removing all those calls so that we can unship that function again and make it impossible to bypass the sanitizer.
Depends on: 1434155
Element::UnsafeSetInnerHTML also introduced the parameter |bool aNeverSanitize| in SetInnerHTMLInternal().
Please remember to remove this parameter as well.

Source: https://searchfox.org/mozilla-central/rev/2aa42f2cab4a110edf21dd7281ac23a1ea8901f9/dom/base/FragmentOrElement.h#271-272
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Comment on attachment 8981244 [details]
Bug 1444394 - Remove Element::UnsafeSetInnerHTML.

https://reviewboard.mozilla.org/r/247324/#review253386

\o/
Attachment #8981244 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8981244 [details]
Bug 1444394 - Remove Element::UnsafeSetInnerHTML.

https://reviewboard.mozilla.org/r/247324/#review254160

::: dom/base/nsContentUtils.h
(Diff revision 1)
>     *
>     * @param aContextNode the node which is used to resolve namespaces
>     * @param aFragment the string which is parsed to a DocumentFragment
>     * @param aReturn the resulting fragment
>     * @param aPreventScriptExecution whether to mark scripts as already started
> -   * @param aSanitize whether the fragment should be sanitized prior to

We should probably document that the fragment will be sanitized if aContextNode is system, right?
Attachment #8981244 - Flags: review?(bzbarsky) → review+
Comment on attachment 8981244 [details]
Bug 1444394 - Remove Element::UnsafeSetInnerHTML.

https://reviewboard.mozilla.org/r/247324/#review254160

> We should probably document that the fragment will be sanitized if aContextNode is system, right?

Good idea, thanks!!
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4cab0dc8b7d9
Remove Element::UnsafeSetInnerHTML. r=bz,kmag
https://hg.mozilla.org/mozilla-central/rev/4cab0dc8b7d9
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.