Closed Bug 1398074 Opened 2 years ago Closed 2 years ago

YouTube Flash embeds rewrite should apply to more domains

Categories

(Core :: Audio/Video: Playback, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: mounir, Assigned: qdot)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file)

See $URL

The embed with youtube-nocookie.com works on Safari and Chrome while Firefox ignores it. Both browsers have open source implementation of the embed rewriter so you might want to check which URLs they use :)
Patch adds support for youtube-nocookie.com. Need to do a little more research to figure out what our situation is on youtu.be. May fix that in a followup if this gets reviewed soon enough.
Comment on attachment 8906104 [details]
Bug 1398074 - Add youtube-nocookie.com to domain list for HTML5 rewrites;

https://reviewboard.mozilla.org/r/177848/#review182900

LGTM

::: dom/base/nsObjectLoadingContent.cpp:1389
(Diff revision 1)
>      return;
>    }
>  
>    // See if URL is referencing youtube
> -  if (!currentBaseDomain.EqualsLiteral("youtube.com")) {
> +  if (!currentBaseDomain.EqualsLiteral("youtube.com") &&
> +      !currentBaseDomain.EqualsLiteral("youtube-nocookie.com")) {

Are there any other YouTube domains we should whitelist, such the "youtu.be" short URL? I don't know if youtu.be videos can be embedded.
Yup, see comment 2. Gonna do a little more research on that. Webkit replaces youtu.be, but I want to make sure that's warranted first, as it'll also require more rewrite logic, and their fix goes farther than ours (they actually build the iframe themselves).
Landing this now (just so I don't get sidetracked before the 57 cutoff, which has been a common occurrence lately) and following up for youtu.be in bug 1398377.
Comment on attachment 8906104 [details]
Bug 1398074 - Add youtube-nocookie.com to domain list for HTML5 rewrites;

https://reviewboard.mozilla.org/r/177848/#review182920
Comment on attachment 8906104 [details]
Bug 1398074 - Add youtube-nocookie.com to domain list for HTML5 rewrites;

https://reviewboard.mozilla.org/r/177848/#review182900

Filed a followup for other domains, can this be r+'d?
I'm marking this as a P2 because it likely breaks a number of sites.
Comment on attachment 8906104 [details]
Bug 1398074 - Add youtube-nocookie.com to domain list for HTML5 rewrites;

https://reviewboard.mozilla.org/r/177846/#review185898

LGTM. I thought I already r+'d this review request, but MozReview insists I haven't.
Attachment #8906104 - Flags: review?(cpeterson)
Comment on attachment 8906104 [details]
Bug 1398074 - Add youtube-nocookie.com to domain list for HTML5 rewrites;

https://reviewboard.mozilla.org/r/177848/#review185900
Attachment #8906104 - Flags: review+
Pushed by kmachulis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5125dbdcbcb2
Add youtube-nocookie.com to domain list for HTML5 rewrites; r=cpeterson
https://hg.mozilla.org/mozilla-central/rev/5125dbdcbcb2
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.