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

fix: Update custom-dialog.js DOM text reinterpreted as HTML #2331

Closed
wants to merge 1 commit into from

Conversation

Shivam7-1
Copy link
Contributor

By using textContent, it will avoid the risk of Dynamic HTML injection, as these properties automatically escape any HTML special characters in the provided text. This helps prevent cross-site scripting (XSS) vulnerabilities by treating the input as plain text rather than interpreted HTML.

@Shivam7-1 Shivam7-1 requested a review from a team as a code owner April 19, 2024 17:46
@Shivam7-1 Shivam7-1 requested review from BeksOmega and removed request for a team April 19, 2024 17:46
Copy link

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@NeilFraser NeilFraser requested review from NeilFraser and removed request for BeksOmega April 19, 2024 18:22
@NeilFraser NeilFraser assigned NeilFraser and unassigned BeksOmega Apr 19, 2024
@NeilFraser
Copy link
Contributor

You need to look at what the code is doing. When projects set .innerHTML with HTML tags, one can't change this to either .textContent or .innerText. Doing so would print raw HTML on the user's screen. There are a dozen such changes you've proposed in your history. Please don't keep proposing this breaking change.

Also (after looking at your other PRs), .innerHTML is absolutely safe when used as a getter. There's never a need to change it unless it is being set.

@NeilFraser NeilFraser closed this Apr 19, 2024
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.

3 participants