Modify-able config.

[otherdaniel]

[This isn't quite ready yet. Using a PR for tool support and discussion.]

This re-writes the config logic, based on discussions in the recent meetings, and with the intent to address issues #228, #229, and #233.

The corner stones are:

• The Sanitizer (and thus its config) become mutable. It has methods to add stuff to any of the allow or deny lists.
• The actual config processing is now defined in terms of those methods.
• There is only one "safe-ify" operation, and it gets called once (for safe methods). This is the only different between safe and unsafe versions.
  • (Well, this is only a half-truth. The handling of javascript:-navigation URLs remains an odd-ball that is magically different between "safe" and "unsafe" versions.)
• The configs have an "other" setting, which allows them to express "everything" and "nothing" filters, which in turn allows us to treat the unsafe defaults as just another regular case.
• The entire "known" logic is gone. But in turn, the "baseline" is now back, and is a deny-list.

---

Preview | Diff

[mozfreddyb]

We discussed this in a synchronous meetings. I'm copying the notes verbatim so we can continue async.

• Document.parseHTML / Document.parseHTMLUnsafe - How does it handle safety?
• We should not need otherMarkup
• get/getUnsafe should not be a method
• Some method for turning unsafe into safe
• Don’t want to call it “safe”, because we don’t know what safe is
• Eg. “WithBaseline” (baseline is applied to config)
• Element vs allowAttribute?
• setComment why set-method instead of an attribute-setter?
• Should we use allow instead of allowElement (or element)
• Anne: How to undo replaceWithChildren
• Tom: element() currently removes from the replaceWithChildren list
• Guillaume: “Other name for something captured by allow or block list”
• We have a baseline know
• Annevk: baseline remove all unsafe stuff
• Default “safe list” of allowed elements
• Annevk: Expose the default list with a static method
• Anne: Name the method removeUnsafe to make a config safe (instead of safeify, because we can’t easily guarantee safeness for all use cases)
• It shouldn't be replaceWithChildren but replaceWithChildrenElements (as per earlier group consensus)
• Anne: Rename methods to addElement, addRemoveElement, addReplaceWithChildrenElement, etc. (prefix with add to make it clear there is a modification of the instance)
• No group concensus for the add prefix. We need to discuss this further.
• Tom: Make setComment a property like comment
• Anne: This would imply introspection for the properties and not for other like elements, removeElements etc.
• Anne: Remove getUnsafe, instead a method that makes sanitizer safe (removeUnsafe)
• Guillaume: get() returns whatever the current state is
• Anne: Could we use toJSON instead of get?
• Guillaume/Frederik: We need to worry about having functions that are not JSON compatible in the future (e.g. if we add a fallback feature in v2)
• all: Let’s keep get then.

[mozfreddyb]

Really great progress so far. We have some nits (and some larger questions) in the review notes, please see above.

[otherdaniel]

• Added issue links to issues for naming and other markup
• Added "unsafe" handling to parseHTMLUnsafe.
• Replaced getUnsafe query method with removeUnsafe modifier.
• Naming mostly deferred. But made sure all method names start with a verb.
• Finished handling of element-dependent attributes.
• Various editorial / link-ification improvements.

[annevk]

Modulo naming the general setup here looks really solid to me. I mainly have editorial concerns. Thanks a lot for your work on this!

[mozfreddyb]

copying over from #238

change

• replaceWithChildrenElements to replaceElementWithChildren()
• setComments() to plural.

keep:

• removeElement()
• allowElement()
• removeAttribute()
• allowAttribute()
• setDataAttributes()

[otherdaniel]

Updated the PR with feedback from today's meeting:

• Adopted the updated naming.
• Introduced canonicalization methods for element with attributes; elements; and attributes. Rewrote the caller algorithms so that they canonicalize in the first step, and then only use the canonicalized value.

[annevk]

At this point I would be okay with landing this to get the public draft in a better state, but we'll need follow-up issues for setHTML() defaulting and such (and any comments in this PR that you decide to postpone addressing).

[annevk]

Do we still need this?

[otherdaniel]

I suspect not. I left this one, since the next step is to re-work the default handling along the lines we discussed in yesterday's meeting, and I hope I can fix all the builtin/default things in one go thhere.

[otherdaniel]

I think I resolved all the issues. Will land tomorrow, if no more comments come in tonight.

Next step will be to rework the default handling, and to put up some doc and/or poposal about the built-in lists.

[mozfreddyb]

🎈