Skip to content

Replace replace all with steps from replaceWith.#375

Open
otherdaniel wants to merge 2 commits intoWICG:mainfrom
otherdaniel:372-replace-all
Open

Replace replace all with steps from replaceWith.#375
otherdaniel wants to merge 2 commits intoWICG:mainfrom
otherdaniel:372-replace-all

Conversation

@otherdaniel
Copy link
Copy Markdown
Collaborator

@otherdaniel otherdaniel commented Mar 4, 2026

Replace [=replace all=] with [=convert nodes into a node=] and [=replace=].

Fixes: #372


Preview | Diff

@evilpie evilpie requested a review from annevk March 4, 2026 14:31
@@ -904,7 +905,9 @@ beginning with |node|. It consistes of these steps:
[=SanitizerConfig/contains=] |elementName|:
1. Call [=sanitize core=] on |child| with |configuration| and
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My assumption was that the top-level list/iterate would have some sort of pass-by-value semantics, i.e. that the list at the time of the iteration is fixed. In that case this is needed, because otherwise the sanitizer would never see any of the children that are replacing their parents.

Spec text says:

To iterate over a list, performing a set of steps on each item in order, [...]

That doesn't strike me as very clear either way.

I also found this discussion, which seems to resolve this in favour of supporting modifications while iterating. But the promised PR from that discussion seems to not have materialized.

Not sure what to do here... by my reading of the current spec texts, I can't find a guarantee that modify during iterate would work as expected.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whatwg/infra#451 exists but seems to have stalled.

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

Successfully merging this pull request may close these issues.

Incorrect use of "replace all"

3 participants