WordPress.org

Make WordPress Core

Opened 15 months ago

Last modified 9 days ago

#44610 assigned enhancement

Allow Youtube-Player to use youtube-nocookie.com URLS to avoid setting cookies.

Reported by: jepperask Owned by: williampatton
Milestone: Future Release Priority: normal
Severity: normal Version: 4.9.7
Component: Embeds Keywords: needs-testing has-patch
Focuses: privacy Cc:
PR Number:

Description

The file "wp-includes/class-wp-customize-manager.php" includes a function "_validate_external_header_video( $validity, $value )". The regex used in this function is incomplete, as some urls are invalidated in the customizer. What is interesting is that the regex used in "wp-includes/js/wp-custom-header.js" (which actually sets the youtube video), is different and validates e.g the youtube-nocookie.com URLs, that I think more people will need due to GDPR.

In the javascript file, it actually quotes a stackoverflow regex found at: http://stackoverflow.com/a/27728417

Proposal:

Update the regex in "wp-includes/class-wp-customize-manager.php" (line 5664) to match the one used in "wp-includes/js/wp-custom-header.js" (line 379).

Attachments (2)

44610.diff (6.4 KB) - added by jepperask 15 months ago.
44610.2.diff (5.2 KB) - added by jepperask 15 months ago.

Download all attachments as: .zip

Change History (16)

#1 @jepperask
15 months ago

  • Resolution set to invalid
  • Status changed from new to closed

#2 @jepperask
15 months ago

Closed as it seems the JS-file needs more work, even if the URL is validated with e.g a youtube-nocookie.com URL.

#3 @netweb
15 months ago

  • Milestone Awaiting Review deleted

#4 @jepperask
15 months ago

  • Keywords needs-testing 2nd-opinion needs-unit-tests added
  • Resolution invalid deleted
  • Status changed from closed to reopened

I've done some more digging and made it show the no-cookie version, however autoplay does not seem consistant.
I'm new to this part of Wordpress - can i submit a pull-request for review and input from others somewhere?

My changes are simply updating the regex (PHP) and adding an argument to the YT.Player initialization (JS). Resolving the 'host' argument is a bit clumsy, so I'd appreciate if someone would take over.

Otherwise here comes a description of my changes:
wp-includes/class-wp-customize-manager.php:5664 - change function to:

public function _validate_external_header_video( $validity, $value ) {
        $video = esc_url_raw( $value );
        if ( $video ) {
                if ( ! preg_match( '#^https?://(?:www\.)?(youtube|youtube-nocookie)\.com/(watch|embed|youtu\.be/)#', $video ) ) {
                        $validity->add( 'invalid_url', __( 'Please enter a valid YouTube URL.' ) );
                }
        }
        return $validity;
}

wp-includes/theme.php:1402 - change line to:

if ( preg_match( '#^https?://(?:www\.)?(youtube|youtube-nocookie)\.com/(watch|embed|youtu\.be/)#', $video_url ) ) {

wp-includes/js/wp-custom-header.js:394 - add "host" argument to YT.Player settings dictionary:

host: location.protocol + this.settings.videoUrl.indexOf("youtube-nocookie") !== -1 ? "//www.youtube-nocookie.com" : "//www.youtube.com",

Also I should note that my original ticket description is a bit off. The JS-regex does not validate anything, it simply retrieves the ID of the Youtube URL.

Last edited 15 months ago by jepperask (previous) (diff)

#5 @jepperask
15 months ago

  • Summary changed from Update regex used for YouTube videos in class-wp-customize-manager.php::_validate_external_header_video() to Allow Youtube-Player to use youtube-nocookie.com URLS to avoid setting cookies.

#6 @jepperask
15 months ago

  • Keywords has-patch added

#7 @birgire
15 months ago

  • Focuses privacy added

Welcome to WordPress Core Trac @jepperask

The scope of the ticket seems to be the YouTube url in the Header Media of the Customizer.

Here's some info on YouTube's Privacy Enhanced Mode:

Privacy Enhanced Mode allows you to embed YouTube videos without using cookies to track viewing
behavior. This means viewing activity isn’t collected to personalize the viewing experience. Instead,
video recommendations are contextual and related to the currently played video. Videos playing in a
Privacy Enhanced Mode embedded player won’t influence the viewer's browsing experience on YouTube.

(The Privacy Enhanced Mode only relates to tracking of viewer behavior, not ads-serving behavior.
To disable tracking for advertising purposes, you can add yourself to the Tag for Child-Directed
Treatment page.)

Note:

If the viewer clicks or taps out of the embed and is redirected to another website or app, 
that website or app may track the viewer’s behavior as per that website’s or app’s policies and terms.
Privacy Enhanced Mode is currently available only for embedded players on websites. Developers will
have to wrap the Privacy Enhanced Mode player into a web-view instance in order to use it in apps.
To use Privacy Enhanced Mode, change the domain for the embed URL in your HTML from 
https://www.youtube.com to https://www.youtube-nocookie.com as shown in the following example:

Before
<iframe width="1440" height="762" 
src="https://www.youtube.com/embed/7cjVj1ZyzyE"
frameborder="0" allow="autoplay; encrypted-media" allowfullscreen></iframe>

After
<iframe width="1440" height="762" src="https://www.youtube-nocookie.com/embed/7cjVj1ZyzyE"
frameborder="0" allow="autoplay; encrypted-media" allowfullscreen></iframe>

Since this is a different domain, network administrators also need to add the domain 
youtube-nocookie.com to their firewall whitelist in addition to youtube.com.


https://support.google.com/youtube/answer/171780?hl=en

---

This seems to be the supported format:

https://www.youtube-nocookie.com/embed/{ID}

I tested various versions (with browser and curl) that did not work:

---

If supporting the Privacy Enhanced Mode is the way to go, should the:

https://www.youtube-nocookie.com/embed/{ID}

be the only supported format for the youtube-nocookie.com urls ?

#8 @birgire
15 months ago

ps: we could also note that it's currently not supported to embed

https://www.youtube-nocookie.com/embed/{ID}

e.g. in the post content and in the Video widget, while

https://www.youtube.com/embed/{ID}

is supported.

#9 @jepperask
15 months ago

Thank you for the feedback. It appears I was missing some parentheses in the javascript code:

host: location.protocol + ((this.settings.videoUrl.indexOf("youtube-nocookie") !== -1) ? "//www.youtube-nocookie.com" : "//www.youtube.com"),

Using my changes to the code, I succesfully got it to play the following URLs (TwentySeventeen setting the Header Media on a fresh install):
http://youtube-nocookie.com/embed/sRrqF8eXs38
http://www.youtube-nocookie.com/embed/sRrqF8eXs38
https://www.youtube-nocookie.com/embed/sRrqF8eXs38
https://www.youtube-nocookie.com/watch/?v=sRrqF8eXs38

I'll test more if needed. If you inspect the JS-file, you will find that the Regex is extracting the video-ID, and regardless of what the original URL was, it will appropriately set the URL to:

(http|https)://(youtube|youtube-nocookie).com/embed/{ID}

Schema is set to location.protocol, as I was unable to set the host argument with https when called from my localhost (http).

Last edited 15 months ago by jepperask (previous) (diff)

@jepperask
15 months ago

#10 @jepperask
15 months ago

The embedded HTML in posts appears to origin from https://www.youtube.com/oembed. Regardless of whether it is a youtube.com or youtube-nocookie.com URL we're trying to embed, the returned HTML is an iframe with src set to a youtube.com URL.

Unless we can find information on how to query youtube-oembed for a youtube-nocookie.com URL as src in the iframe, I believe our only choice is to string-replace the src. I have succesfully embedded a youtube-nocookie.com video this way, without any cookies being set.

@jepperask
15 months ago

#11 @swissspidy
10 months ago

  • Component changed from Customize to Embeds
  • Keywords needs-patch added; 2nd-opinion needs-unit-tests has-patch removed
  • Milestone set to 5.1

#12 @pento
9 months ago

  • Milestone changed from 5.1 to Future Release

This needs testing and a decision.

This ticket was mentioned in Slack in #core by jeroenrotty. View the logs.


7 months ago

#14 @williampatton
9 days ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to williampatton
  • Status changed from reopened to assigned

I'm gonna pick this one up as I'd like to see some movement on it. I'll do some testing on the patch when I am able.

@jepperask thank you for providing the initial patches for this approach. On a scan it seems like an ok way to handle it, do we need those changes to the packages.lock file though to make this happen?

Note: See TracTickets for help on using tickets.