Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#32069 closed enhancement (fixed)

Press This: Check embeds list against oembed providers too.

Reported by: stephdau's profile stephdau Owned by: stephdau's profile stephdau
Milestone: 4.3 Priority: normal
Severity: normal Version: 4.2
Component: Press This Keywords: needs-patch
Focuses: Cc:

Description (last modified by stephdau)

We brought the concept of checking pressed URLs against core's list of oembed providers, which makes it possible to auto-embed media in the editor on page load, when scanning a Youtube page, etc.

The check currently resides in the get_suggested_content() method. We should widen the scope of this check to the _limit_embed() method, so we can do away with the tests that check the sent $src against page regexes (not iframe ones):

		} else if ( ! preg_match( '/\/\/(m|www)\.youtube\.com\/watch\?/', $src )          // Youtube video page (www or mobile)
		            && ! preg_match( '/\/youtu\.be\/.+$/', $src )                         // Youtu.be video page
		            && ! preg_match( '/\/\/vimeo\.com\/[\d]+$/', $src )                   // Vimeo video page
		            && ! preg_match( '/\/\/(www\.)?dailymotion\.com\/video\/.+$/', $src ) // Daily Motion video page
		            && ! preg_match( '/\/\/soundcloud\.com\/.+$/', $src )                 // SoundCloud audio page
		            && ! preg_match( '/\/\/twitter\.com\/[^\/]+\/status\/[\d]+$/', $src ) // Twitter status page
		            && ! preg_match( '/\/\/vine\.co\/v\/[^\/]+/', $src ) ) {              // Vine video page
			$src = '';
		}

Once this is done, we could have the get_embeds() method also check the page URL, which would then have the embed that is auto-inserted in the editor also be part of the list of suggested media. This way, if someone lands on, say, a TED page, they will have both the auto-inserted embed, as well as a thumbnail to click, should they need it.

Currently, the alternative would be to re-press the page to get the embed again.

If we go that route, we'll also need to tweak the related code in the bookmarklet.

	if ( href.match( /\/\/(www|m)\.youtube\.com\/watch/ ) ||
		href.match( /\/\/vimeo\.com\/(.+\/)?([\d]+)$/ ) ||
		href.match( /\/\/(www\.)?dailymotion\.com\/video\/.+$/ ) ||
		href.match( /\/\/soundcloud\.com\/.+$/ ) ||
		href.match( /\/\/twitter\.com\/[^\/]+\/status\/[\d]+$/ ) ||
		href.match( /\/\/vine\.co\/v\/[^\/]+/ ) ) {

		add( '_embeds[]', href );
	}

I'd do away with it altogether.

Attachments (2)

Screen Shot 2015-04-22 at 16.44.17.png (1.3 MB) - added by stephdau 8 years ago.
Currently: media is auto-embedded, but not suggested in list.
32069.diff (8.0 KB) - added by stephdau 8 years ago.

Download all attachments as: .zip

Change History (7)

#1 @stephdau
8 years ago

  • Summary changed from Check embeds list against oembed providers too. to Press This: Check embeds list against oembed providers too.

#2 @stephdau
8 years ago

  • Description modified (diff)

@stephdau
8 years ago

Currently: media is auto-embedded, but not suggested in list.

#3 @stephdau
8 years ago

  • Description modified (diff)

@stephdau
8 years ago

#4 @stephdau
8 years ago

  • Owner set to stephdau
  • Status changed from new to accepted

32069.diff implements the proposed solution.

#5 @azaozz
8 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 32828:

Press This: Check the embeds list against all of the oembed providers.
Props stephdau. Fixes #32069.

Note: See TracTickets for help on using tickets.