Skip to content

fix(SNOTES-480): display tag title as string in delete dialog#2904

Merged
amanharwara merged 6 commits into
mainfrom
fix/display-tag-title-as-string-in-delete-dialog
Jun 18, 2025
Merged

fix(SNOTES-480): display tag title as string in delete dialog#2904
amanharwara merged 6 commits into
mainfrom
fix/display-tag-title-as-string-in-delete-dialog

Conversation

@antsgar

@antsgar antsgar commented Jun 17, 2025

Copy link
Copy Markdown
Contributor

In the SKAlert component, both the title and the message of the dialog are being interpolated within the rest of the HTML, and then all of that HTML is set as innerHTML for the corresponding element. This means if the title or the message contain HTML themselves, it will be rendered within the dialog. This PR changes the implementation so that the initial template will include empty elements where the title and message will then be set using textContent, so that they're always displayed as strings.

Updated:
Because there are a few places where we intentionally use HTML within the strings for a dialog, I decided to go in a different direction and escape the HTML for notes/tags/files titles.

  • Ultimately it could be better to move away from passing HTML to the alerts because there's a risk that we might forget to escape strings for user-created entities in the future again.
  • I'm not entirely sure I covered all the possible cases of user-created HTML strings that should be escaped.
  • When possible I updated to use functions from StringUtils. Having all of the strings in that one file should make it easier to spot cases where we're not escaping when needed.

@antsgar antsgar requested review from amanharwara and atmoio June 17, 2025 14:46
@amanharwara amanharwara merged commit e225060 into main Jun 18, 2025
7 of 8 checks passed
@antsgar antsgar deleted the fix/display-tag-title-as-string-in-delete-dialog branch June 18, 2025 17:47
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.

2 participants