Skip to content

Fix YouTube error 153#217

Merged
mgrdcm merged 4 commits intojazzband:masterfrom
Letme:patch-1
Feb 17, 2026
Merged

Fix YouTube error 153#217
mgrdcm merged 4 commits intojazzband:masterfrom
Letme:patch-1

Conversation

@Letme
Copy link
Contributor

@Letme Letme commented Oct 30, 2025

I got YouTube error 153 on my embedded players that went away after I added referrerpolicy="strict-origin-when-cross-origin" to the template of embed video. This was suggested in few StackOverflow questions and it is based on https://developers.google.com/youtube/terms/required-minimum-functionality#embedded-player-api-client-identity

I got YouTube error 153 on my embedded players that went away after I added `referrerpolicy="strict-origin-when-cross-origin"` to the template of embed video. This was suggested in few StackOverflow questions and it is based on https://developers.google.com/youtube/terms/required-minimum-functionality#embedded-player-api-client-identity
Copy link

@citizenfish citizenfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this and it works well

@aleksihakli
Copy link
Member

Hey @Letme, could you fix the unit tests so we can get this merged?

@Letme
Copy link
Contributor Author

Letme commented Feb 3, 2026

Hey @Letme, could you fix the unit tests so we can get this merged?

Sure. I hope CI will now work as I push this (was working with WebIDE).

@Letme
Copy link
Contributor Author

Letme commented Feb 3, 2026

@aleksihakli I pushed the changes. Locally tox ran fine.

@mgrdcm mgrdcm self-requested a review February 6, 2026 17:15
Copy link
Member

@mgrdcm mgrdcm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just have questions about whether this is also needed for Vimeo?

'<iframe width="80%" height="30%" '
'src="https://player.vimeo.com/video/72304002" '
'loading="lazy" frameborder="0" allowfullscreen></iframe>',
'loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed for Vimeo too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not find any information for it. Should this be a blocker?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just mean that I think this test should probably fail because it's checking for the referrer policy for Vimeo and not for YouTube.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'<iframe width="80%" height="300" '
'src="https://player.vimeo.com/video/72304002" '
'loading="lazy" frameborder="0" allowfullscreen></iframe>',
'loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed for Vimeo too?

Copy link
Member

@mgrdcm mgrdcm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new referrerpolicy is getting added for all backends rather than just YouTube, which I don't think is the intention or desirable. It should be possible to add it in a way that it only applies to YouTube, happy to help with that!

@@ -1 +1 @@
<iframe width="{{ width }}" height="{{ height }}" src="{{ backend.url }}" loading="lazy" frameborder="0" allowfullscreen></iframe>
<iframe width="{{ width }}" height="{{ height }}" src="{{ backend.url }}" loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting added for all backends rather than just YouTube, which I don't think is the intention or desirable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For instance could add a variable to the default embed_code.html and the YouTube backend either uses a template that inherits from that one, or maybe just the code for the YT backend fills in that variable.

Copy link
Contributor Author

@Letme Letme Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain me why this would not be good for Vimeo? What would break by explicitly defining it?

Copy link
Contributor Author

@Letme Letme Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I ping on this as a decision? Because if you want me to put a backend around then fine, but I need the changes from official release, as now I have to manually fix the template post installation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Letme Hello! Sorry to be a holdup on this, and thanks to you and @aleksihakli for the feedback and patience.

My worry was that it would be leaking more information to the host of the video than before, but now I'm not quite sure. According to W3C no-referrer-when-downgrade is the default, while MDN says it's no-referrer-when-downgrade. Looks like Chrome changed defaults from the former to the latter many years ago and so I'm guessing at least some other browsers have as well.

So it looks like this isn't even much of a change in most cases, which makes me wonder why it works/is necessary, but I don't see a reason to hold it up anymore at least.

Thanks for this change @Letme!

@aleksihakli
Copy link
Member

aleksihakli commented Feb 9, 2026

Does this have the potential to introduce any unwanted side-effects?

I'd see this as a purely improvement unless it degrades something or I'm missing something - this simply disables the sending of the Referer policy when the origin domain changes, so the video sites do not get the embedding site URL?

Maybe this could be included with a flag that would default to True, so users could disable the property and send the header if they so desire?

What do you think @mgrdcm?

@mgrdcm mgrdcm self-requested a review February 16, 2026 20:27
@mgrdcm mgrdcm requested a review from aleksihakli February 16, 2026 20:28
@codecov
Copy link

codecov bot commented Feb 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.10%. Comparing base (b9e6997) to head (0d2c4a4).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #217   +/-   ##
=======================================
  Coverage   99.10%   99.10%           
=======================================
  Files          16       16           
  Lines         671      671           
  Branches       40       40           
=======================================
  Hits          665      665           
  Misses          4        4           
  Partials        2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mgrdcm mgrdcm merged commit 07acd44 into jazzband:master Feb 17, 2026
24 checks passed
@Letme
Copy link
Contributor Author

Letme commented Feb 17, 2026

Thanks for merging. Please also make a release so that we can install the fix via pypi.

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.

4 participants

Comments