Fix chapter name XSS injection in progress bar

[nielsvanvelzen]

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

• Fix chapter name XSS injection in progress bar

Issues
Fixes #5561

[dmitrylyzo]

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.

[nielsvanvelzen]

Unfortunately, even if this was meant to change an icon it still doesn't work great. This screenshot was posted in our troubleshooting channel:

It looks terrible

[dmitrylyzo]

It looks terrible

If necessary, we can add the icon as a child element to the marker (wrap current "span"). Then className (applied to the marker) can be provided with some padding/margin/position.

I mean, the idea was to provide some way for marker customization.

[thornbill]

The material-icons class should probably be removed also. If we want to keep the title then it could be added as a data attribute... there is no reason to add it as a class.

[dmitrylyzo]

The material-icons class should probably be removed also.

Yes, it's useless in its current state.

If we want to keep the title then it could be added as a data attribute... there is no reason to add it as a class.

className is not a chapter title (title is name). As I said, its purpose is to allow some customization. Currently, it looks like you can only change background-color (say "gray" for chapters, "yellow" for opening/ending).
Looks like we need to add another element to be able to support icons.

Or do you mean storing the title in addition to the class?

[nielsvanvelzen]

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.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[jellyfin-bot]

Cloudflare Pages deployment

Latest commit	b8a7cf2
Status	✅ Deployed!
Preview URL	https://65011071.jellyfin-web.pages.dev
Type	🔀 Preview

View build logs
View bot logs

[thornbill]

className is not a chapter title (title is name). As I said, its purpose is to allow some customization. Currently, it looks like you can only change background-color (say "gray" for chapters, "yellow" for opening/ending). Looks like we need to add another element to be able to support icons.

Or do you mean storing the title in addition to the class?

I mean we should not be appending the chapter Name as an additional css class (marker-${markerInfo.name.substring(0, 100).toLowerCase().replace(' ', '-')}). Maybe we could add it as a data-* attribute, but since it is unused, I would prefer just removing it for now.