Fix YouTube error 153 #217
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| <iframe width="{{ width }}" height="{{ height }}" src="{{ backend.url }}" loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe> | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,7 +61,7 @@ def test_embed(self): | |
| template, | ||
| '<iframe width="960" height="720" ' | ||
| 'src="https://www.youtube.com/embed/jsrRJyHBvzw?wmode=opaque" ' | ||
| 'loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>', | ||
| ) | ||
|
|
||
| def test_embed_invalid_url(self): | ||
|
|
@@ -97,7 +97,7 @@ def test_direct_embed_tag(self): | |
| template, | ||
| '<iframe width="960" height="720" ' | ||
| 'src="https://www.youtube.com/embed/jsrRJyHBvzw?wmode=opaque" ' | ||
| 'loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>', | ||
| ) | ||
|
|
||
| def test_direct_embed_tag_with_default_size(self): | ||
|
|
@@ -109,7 +109,7 @@ def test_direct_embed_tag_with_default_size(self): | |
| template, | ||
| '<iframe width="480" height="360" ' | ||
| 'src="https://www.youtube.com/embed/jsrRJyHBvzw?wmode=opaque" ' | ||
| 'loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>', | ||
| ) | ||
|
|
||
| def test_direct_embed_invalid_url(self): | ||
|
|
@@ -130,7 +130,7 @@ def test_user_size(self): | |
| template, | ||
| '<iframe width="800" height="800" ' | ||
| 'src="https://www.youtube.com/embed/jsrRJyHBvzw?wmode=opaque" ' | ||
| 'loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>', | ||
| ) | ||
|
|
||
| def test_wrong_size(self): | ||
|
|
@@ -233,7 +233,7 @@ def test_relative_size(self): | |
| template, | ||
| '<iframe width="80%" height="30%" ' | ||
| 'src="https://player.vimeo.com/video/72304002" ' | ||
| 'loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>', | ||
|
Contributor
Author
There was a problem hiding this comment. I did not find any information for it. Should this be a blocker?
Member
There was a problem hiding this comment. 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.
Contributor
Author
There was a problem hiding this comment. I found similar thing at Vimeo, so I am a bit more certain this can help it: https://help.vimeo.com/hc/en-us/articles/35817429341457-Troubleshooting-video-playback-errors-due-to-referrer-policy-conflicts |
||
| ) | ||
|
|
||
| def test_allow_spaces_in_size(self): | ||
|
|
@@ -245,7 +245,7 @@ def test_allow_spaces_in_size(self): | |
| template, | ||
| '<iframe width="80%" height="300" ' | ||
| 'src="https://player.vimeo.com/video/72304002" ' | ||
| 'loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>', | ||
|
|
||
| ) | ||
|
|
||
| def test_embed_with_query(self): | ||
|
|
@@ -285,7 +285,7 @@ def test_direct_embed_with_query(self): | |
| output_without_url, | ||
| '<iframe width="480" height="360" ' | ||
| 'src="URL" ' | ||
| 'loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>', | ||
| ) | ||
|
|
||
| def test_set_options(self): | ||
|
|
@@ -297,7 +297,7 @@ def test_set_options(self): | |
| template, | ||
| '<iframe width="300" height="200" ' | ||
| 'src="https://www.youtube.com/embed/jsrRJyHBvzw?rel=1" ' | ||
| 'loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>', | ||
| ) | ||
|
|
||
| def test_size_as_variable(self): | ||
|
|
@@ -311,7 +311,7 @@ def test_size_as_variable(self): | |
| template, | ||
| '<iframe width="500" height="200" ' | ||
| 'src="https://www.youtube.com/embed/jsrRJyHBvzw?wmode=opaque" ' | ||
| 'loading="lazy" frameborder="0" allowfullscreen referrerpolicy="strict-origin-when-cross-origin"></iframe>', | ||
| ) | ||
|
|
||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 is getting added for all backends rather than just YouTube, which I don't think is the intention or desirable.
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.
For instance could add a variable to the default
embed_code.htmland 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.Uh oh!
There was an error while loading. Please reload this page.
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.
Can you explain me why this would not be good for Vimeo? What would break by explicitly defining it?
Uh oh!
There was an error while loading. Please reload this page.
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.
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.
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.
@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-downgradeis the default, while MDN says it'sno-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!