Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should the sanitizer normalize #203

Closed
annevk opened this issue Nov 20, 2023 · 6 comments
Closed

Should the sanitizer normalize #203

annevk opened this issue Nov 20, 2023 · 6 comments
Milestone

Comments

@annevk
Copy link
Collaborator

annevk commented Nov 20, 2023

See #202 for context. Someone(tm) should probably look at what ranges do as they have similar multi-node mutation operations.

@annevk annevk added the v1 label Nov 20, 2023
@otherdaniel
Copy link
Collaborator

Ranges apparently don't normalize, or at least Range.insertNode doesn't.

spec: https://dom.spec.whatwg.org/#concept-range-insert

impl:

d = document.createElement("div");
d.textContent = "abcdef";
d.childNodes.length   // 1, as expected

r = document.createRange();
r.setStart(d.firstChild, 3);  // position range in the middle of the text node.
r.insertNode(document.createTextNode("XXX"));
d.innerHTML  // "abcXXXdef"
d.childNodes.length  // 3

@mozfreddyb
Copy link
Collaborator

I just can't really think of any pros & cons. In case of setHTML(), when starting with a string, I don't think there's particular need to preserve adjacent text nodes as individual items and combining them could save a bit of memory, maybe.

On the other hand, doing a full normalization pass after or during sanitization is a bit of extra work that just not be necessary.

I don't have strong feelings either way. From what I can tell, there is no other API really normalizing. Given this, I can't think of a reason why this one should.

@annevk
Copy link
Collaborator Author

annevk commented Dec 2, 2023

@smaug---- @rniwa thoughts?

@smaug----
Copy link

My initial feeling is that normalization would be just a bit odd, even unexpected size effect here, given that other APIs don't tend to normalize.

@mozfreddyb
Copy link
Collaborator

Without anyone being of a strong opinion and the web platform mostly not normalizing, we will also just not normalize.

@mozfreddyb mozfreddyb added this to the v1 milestone Jan 23, 2024
@mozfreddyb
Copy link
Collaborator

I guess this is resolved and does not need any further action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants