Configuration edge cases

[otherdaniel]

In the last meeting, and in discussing #196, a number of interesting edge cases came up, where it's unclear how to handle them. There didn't seem to be any strong opinions on what the best solutions might be; but we'll need to agree on some mode for compatibility.

Mixing allow w/ block and drop

{ allowElements: [ "a", "b" ], blockElements: ["a", "c"] }
{ allowElements: [ "a", "b" ], dropElements: ["a", "c"] }

In the meeting, the preferred interpretation was that this would allow only "b" elements. The second would allow "b", and preserve text children of "a" and "c".

(The rule implied here would be that all clauses are processed: Even if allowElements allows an element, an additional blockElements might still block it.)

Repeating clauses for the same element

The previous design used dictionaries which have only one entry per key. This has changed to arrays. Now there can be multiple entries for the same element. This gets interesting with attributes that depend on the element.

{ allowElements: [ { name: "a", attributes: ["b"] },  { name: "a", attributes: ["c"] } ] }

This could mean:

• "a" element is allowed, but neither are "b" or "c" attributes.
  • The implied rule would be that there is at least one clause forbidding either attribute.
• "a" element is allowed, and both "b" and "c" are
  • The implied rule would be that there is at least one clause that allows those attributes.
• "a" with the "b" element is allowed, but "c" isn't.
  • The implied rule is that the first matching rule counts.
• An invalid config, and the method is expected to throw.

In the meeting we weren't sure, with a mild preference emerging for the second choice.

Block-clauses with element-dependent attributes.

Does a blockElements-clause with attributes block the element, too, or only the attributes?

{ blockElements: [ { name: "a", attributes: ["b"] } ] }

Does this allow element "a" or not? Would <a b="1" c="2"> result in <a c="2">, or in an empty sub-tree?

null and undefined attributes.

Are these the same?

{ allowElements: [ { name: "a", attributes: [] } ] }
{ allowElements: [ { name: "a", attributes: null } ] }
{ allowElements: [ { name: "a", attributes: undefined } ] }
{ allowElements: [ { name: "a" } ] }

Debuggability, testability, and introspection

Mario raised the point that - especially if the rules aren't entirely obvious - we should add something to let developers write meaningful tests for their configs.

A good way to add this might be to define a 'normalized' config in the spec, and return a normalized config to the developer. Proposed properties of the 'normal config' could be:

1, always use name + namespace, and
2, at most one clause for each element or attribute name, and
3, based on an allow-list, and
4, remove entries not allowed by the default config.

In other words: Resolve short forms + namespaces; resolve duplicate clauses for any element or attribute; and resolve the default config (for block/drop-clauses).

The idea of 4 is to e.g. remove "script" from allowElements. But since the config can now be used for both setHTML and setHTMLUnsafe, maybe the normalized-config-getter needs two variants, too.

[benbucksch]

Block vs. drop vs. allow

See my comment in #199

Attributes for specific element and for all elements

We need the ability to allow (or block) attributes either for all elements, or only for specific elements. E.g. I might want to work with an allow list, but allow the id attribute on all elements. It would be cumbersome to have to list all elements explicitly. OTOH, I might want to allow or block src attribute only on specific elements, e.g. I want to allow <img src="">, but not <video src="">. So, whatever syntax is chosen, there needs to be a wildcard for this attribute on all elements, or only on a specific element, for both block and allow lists.

Repeated element specs

If the author lists an element multiple times in an allow list, I would merge the lists. E.g. foo(bar,baz) foo(bal) is logically the same as foo(bar,baz,bal). However, I think that such a case is basically a mistake by the author and not really important either way.

[mozfreddyb]

This should be answered by #199, I believe

[annevk]

@otherdaniel can this be closed?