-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 chapter name XSS injection in progress bar #5563
Fix chapter name XSS injection in progress bar #5563
Conversation
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.
I originally only asked for className
(can be used to specify an icon from the Material Icons). It must be defined beforehand by the marker provider.
If necessary, we can add the icon as a child element to the marker (wrap current "span"). Then I mean, the idea was to provide some way for marker customization. |
The |
Yes, it's useless in its current state.
Or do you mean storing the title in addition to the class? |
b9308ab
to
b8a7cf2
Compare
Updated the PR, I've now also removed the material-icons class and the className option as it is never used in any CSS. If we want to show icons again I personally consider that a feature that should not be in a backport. |
Quality Gate passedIssues Measures |
Cloudflare Pages deployment
|
I mean we should not be appending the chapter |
Fix chapter name XSS injection in progress bar Original-merge: 7eb54e0 Merged-by: thornbill <[email protected]> Backported-by: Joshua M. Boniface <[email protected]>
The chapter markers in the video player seekbar, introduced in 10.9, have some issues. First they add unsanitized user values to the seekbar (chapter name). And secondly, those names can be anything which could cause serious issues when they clash with existing CSS classes. I have no idea why we add these names as a class as this does not provide any benefit. For now, just add the specific className (which is currently always
chapterMarker
).Changes
Issues
Fixes #5561