search

hypothesis / product-backlog

Where new feature ideas and current bugs for the Hypothesis product live
118 stars 7 forks source link

What differentiates embeddable YT videos from non-embeddable YT videos that say they are embeddable? #1297

Closed mkdir-washington-edu closed 2 years ago

mkdir-washington-edu commented 2 years ago

Bug report form

Steps to reproduce

  1. Embed a YT video in an annotation.
  2. Now embed https://www.youtube.com/watch?v=gJLIiF15wjQ, note that it is embeddable
  3. See error

Expected behaviour

Embeddable videos should embed in annotations.

Additional details

Slack: https://hypothes-is.slack.com/archives/C2BLQDKHA/p1640961903389000 User video showing this not working: https://www.youtube.com/watch?v=IcQO20GEbuc User etherpad detailing what they tried: https://etherpad.wikimedia.org/p/Annotation_links_for_YT_video_unavailable Initial tweet: https://twitter.com/telliowkuwp/status/1476566176457281542

I suspect there's an issue with monetized videos, even when a video is embeddable.

robertknight commented 2 years ago

I tried embedding https://www.youtube.com/watch?v=gJLIiF15wjQ in an annotation and it worked for me in Safari, Chrome and Firefox. If the issue is related to monetization then it is possible the user's location matters.

robertknight commented 2 years ago

I just tried all three videos mentioned in https://etherpad.wikimedia.org/p/Annotation_links_for_YT_video_unavailable and was not able to reproduce the issue. I tested in Chrome 99, Safari 15 and Firefox 95.

robertknight commented 2 years ago

Slack thread investigating the issue here: https://hypothes-is.slack.com/archives/C2BLQDKHA/p1642510579173500

@mkdir-washington-edu and @lyzadanger were able to reproduce it, but there were inconsistencies in outcomes. I was not able to reproduce. For example Lyza reported a case where a video embedded in a single-annotation page worked but on the same annotation in-context it did not. This seems similar to an issue reported in WordPress at https://wordpress.org/support/topic/embedded-youtube-issue-video-unavailable/. In that case the author said that the failure was triggered when an embedded video was detected to contain copyrighted content (eg. an audio track).

If embedding is explicitly disabled for a video, a different error is shown:

Embedding disallowed

In any case, it seems very likely that the issue is out of our control. I don't think we have a way to detect the failure from within the Hypothesis client. I suggest we create a knowledge base article that acknowledges the problem and says something like "YouTube may prevent videos which contain copyrighted content from playing directly within annotations."

robertknight commented 2 years ago

Correction to my previous comment. It seems the issue relates to whether the annotation is viewed in the embedded client or the browser extension. In all my tests I'd been viewing the annotation in the embedded client. When I tried in the browser extension I saw this error:

Embed error in extension

When the YouTube iframe HTML loads, different headers get sent in both cases. In particular in the Chrome extension case, no Referer header is set, which matters according to https://stackoverflow.com/questions/51424578/embed-youtube-code-is-not-working-in-html.

robertknight commented 2 years ago

The issue can be reproduced by saving the following HTML and viewing it using a local server.

  1. Save the below as player-test.html
<html>
  <body>
    <iframe src="https://www.youtube.com/embed/orJSJGHjBLI" referrerpolicy="no-referrer">
  </body>
</html>
  1. Run python3 -m http.server 8006
  2. Access http://localhost:8006/player-test.html and see the error
  3. Delete the referrerpolicy attribute from the iframe and reload the page. The video will now load.
robertknight commented 2 years ago

Possibly related upstream issue: https://bugs.chromium.org/p/chromium/issues/detail?id=992700

robertknight commented 2 years ago

Various threads in the Chrome extensions development mailing list suggest that it is expected behavior that no Referer header is included on resource requests from HTML pages served from a Chrome extension. eg. from this thread:

Chrome doesn't send any data to the server that could be used to identify whether the request comes from a browser extension. There won't be any header whatsoever that you can check. In general, cross-origin requests initiated by extensions look to the server as if they were initiated by typing the URL in the address bar (i.e. no origin and no referer).

Some ideas I can think of as a workaround:

  1. Use Chrome extension header modification APIs to modify requests coming from the browser extension and add a Referer header. I would only opt for this if we can do so without asking for additional permissions.
  2. Create a proxy page on the https://hypothes.is or https://cdn.hypothes.is/ domains which embeds the YouTube iframe. This proxy will add the Redirect header, at the cost of creating an additional iframe.

There is an old explanation of why there is no Referer header here:

The Referer only takes a few whitelisted schemes, such as http and https. We don't send chrome-extension because it's not a "real" URL.

It looks like this restriction still applies.

robertknight commented 2 years ago

I have a proof of concept for an ugly workaround that does not require additional extension permissions and should work equally well with Manifest V2 and Manifest V3. See https://github.com/hypothesis/client/pull/4125. It would probably be wise to file an issue with Chrome in case anyone has a better suggestion.

robertknight commented 2 years ago

Additional related issues:

https://bugs.chromium.org/p/chromium/issues/detail?id=429556 https://bugs.chromium.org/p/chromium/issues/detail?id=81923

robertknight commented 2 years ago

I filed a Chrome issue about the general issue with subresources and Referer headers on the off-chance I can persuade the Chrome extensions team to consider a solution: https://bugs.chromium.org/p/chromium/issues/detail?id=1291898.

lyzadanger commented 2 years ago

To move this forward, the next step is to make a decision:

mkdir-washington-edu commented 2 years ago

of course @chrisshaw should weigh in, but from a Support angle I'd be happy with documenting why this doesn't work and seeing if Chrome addresses the issue down the line.

chrisshaw commented 2 years ago

@lyzadanger @robertknight can you explain the trade-offs? Seems like:

What else?

Let's wait for the Chromium team to triage it, at least. Assuming they aren't going to change anything, I would expect us to need to solve this, but let's put a dependency on where they triage it.

@mkdir-washington-edu that would mean putting together that documentation. How often does this come up? Seems like a lot, but that's recency bias. It theoretically seems like it should come up a lot though.

robertknight commented 2 years ago

@lyzadanger @robertknight can you explain the trade-offs? Seems like:

My main concern is workaround implementation complexity / maintenance costs for something that has inconvenienced very few users in practice. This issue:

In addition to the maintenance overhead there will be a performance and memory usage impact of putting every YouTube video in a wrapper frame.

One possible outcome of this issue is that we decide not to proceed with the workaround at this time, but let this issue provide a record of the investigation and revisit it if:

chrisshaw commented 2 years ago

I would suggest we close as Will Not Fix for the reason described (bug filed with Chrome team). And then write up an article explaining why you may get these errors (Browser extension on copyrighted content). @mkdir-washington-edu want to take point?

In case it's convenient: the relevant Slack thread for the KB

mkdir-washington-edu commented 2 years ago

Matt and I updated the documentation. See: https://hypothes-is.slack.com/archives/C2BLQDKHA/p1644600837390399.

Ok to close @chrisshaw @robertknight ?

robertknight commented 2 years ago

Good with me.