[tracking] Remove Element::UnsafeSetInnerHTML

RESOLVED FIXED in Firefox 62

Status

()

P3
normal
RESOLVED FIXED
a year ago
8 days ago

People

(Reporter: johannh, Assigned: johannh)

Tracking

({meta})

unspecified
mozilla62
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
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.
(Assignee)

Updated

a year ago
Depends on: 1444395
(Assignee)

Updated

a year ago
Depends on: 1444428
(Assignee)

Updated

a year ago
Depends on: 1444436
(Assignee)

Updated

a year ago
Depends on: 1444439
(Assignee)

Updated

a year ago
Depends on: 1444441
(Assignee)

Updated

a year ago
Depends on: 1444443
(Assignee)

Updated

a year ago
Depends on: 1444445

Updated

a year ago
Depends on: 1434155
(Assignee)

Updated

a year ago
Depends on: 1450315
(Assignee)

Updated

11 months ago
Depends on: 1455330
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)

Updated

10 months ago
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 5

10 months ago
mozreview-review
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 6

10 months ago
mozreview-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 hidden (mozreview-request)
(Assignee)

Comment 8

10 months ago
mozreview-review-reply
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!!

Comment 9

10 months ago
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4cab0dc8b7d9
Remove Element::UnsafeSetInnerHTML. r=bz,kmag

Comment 10

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4cab0dc8b7d9
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.