Skip to content

Don’t clone node in handleRawHtml() #47

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

Merged
merged 1 commit into from
Jul 14, 2025
Merged

Don’t clone node in handleRawHtml() #47

merged 1 commit into from
Jul 14, 2025

Conversation

lucaswerkmeister
Copy link
Member

I don’t know why the code cloned the node before modifying it – there’s no explanation of this in #2, and the rest of the code is happy to modify nodes directly. However, this causes attribute binding on nodes with v-html to have no effect – after handleRawHtml(), the original node is skipped for further processing (isRemovedFromTheDom() returns true), and even if it wasn’t, handling attributes on it wouldn’t have any effect on the node that’s actually in the DOM now. Given that removing the cloning doesn’t seem to break any tests, let’s just remove it and add a test for attribute binding in conjunction with v-html.

Bug: T396633
Bug: T396634

I don’t know why the code cloned the node before modifying it – there’s
no explanation of this in #2, and the rest of the code is happy to
modify nodes directly. However, this causes attribute binding on nodes
with v-html to have no effect – after handleRawHtml(), the original node
is skipped for further processing (isRemovedFromTheDom() returns true),
and even if it wasn’t, handling attributes on it wouldn’t have any
effect on the node that’s actually in the DOM now. Given that removing
the cloning doesn’t seem to break any tests, let’s just remove it and
add a test for attribute binding in conjunction with v-html.

Bug: T396633
Bug: T396634
Copy link
Contributor

@codders codders left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me - let's do it!

@codders codders merged commit 5b6318a into master Jul 14, 2025
8 checks passed
@lucaswerkmeister lucaswerkmeister deleted the vHtmlAndBind branch July 14, 2025 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants