-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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(menu-item): Replace innerHTML of menu item to fix string bug on player #8793
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8793 +/- ##
==========================================
- Coverage 83.75% 83.03% -0.73%
==========================================
Files 120 120
Lines 8022 8034 +12
Branches 1925 1930 +5
==========================================
- Hits 6719 6671 -48
- Misses 1303 1363 +60 ☔ View full report in Codecov by Sentry. |
src/js/menu/menu-item.js
Outdated
@@ -74,6 +74,15 @@ class MenuItem extends ClickableComponent { | |||
textContent: this.localize(this.options_.label) | |||
}); | |||
|
|||
const containsHexCode = (s) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a utility function that pertains to strings in particular so I'm wondering if it might be better to move it here.
Since we're proposing using innerHTML as part of this solution, and this function is essentially guarding that step, maybe it would be beneficial to add some tighter conditions to make it more difficult to use other problematic characters (i.e.: /'"
, etc) that are typically escaped for safety? We should also probably add a check for a semicolon ;
as part of the regex test.
Does the menu item label absolutely need to accept an HTML string? Using |
… certain problematic characters
…ccept characters that could be present issues
In terms of security the regex that is used in the new function 'containsHexCode' was updated and will not accept any problematic characters only characters from specific ranges. |
Description
Replace innerHTML of menu item to fix string bug on player.
Specific Changes proposed
With a condition using a regex replace innerHTML of menu item to fix string bug on player when content of the string contains HexCode.
Requirements Checklist
npm run docs:api
to error