WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 15 months ago

#15490 closed enhancement (fixed)

Preview oEmbed results when using the media modal to insert from URL

Reported by: filosofo Owned by:
Milestone: 4.0 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch
Focuses: Cc:

Description

If you insert a video URL via the "Add media file from URL" popup, it should do the following:

  • Determine whether the URL is oEmbed-able, and if so, insert the appropriate shortcode into the post.
  • Create a corresponding attachment with something indicating that it's a video in the post_mime_type field (even though we're not really dealing with true MIME types). That way, we can query video attachments, agnostic of where the actual video file exists.

Attachments (5)

15490.diff (5.3 KB) - added by jtsternberg 3 years ago.
This successfully displays an oembed preview when an oembed url is detected. Still needs a proper nonce passed for the ajax action.
15490.2.diff (5.0 KB) - added by helen 3 years ago.
15490.3.diff (4.1 KB) - added by wonderboymusic 16 months ago.
15490.4.diff (4.2 KB) - added by jtsternberg 16 months ago.
Removes width/height inputs, and improves the styling of the oEmbed items in responsive views.
15490.5.diff (4.5 KB) - added by wonderboymusic 16 months ago.

Download all attachments as: .zip

Change History (40)

comment:1 @jtsternberg3 years ago

  • Cc justin@… added
Version 0, edited 3 years ago by jtsternberg (next)

comment:2 @jtsternberg3 years ago

A feature discussed in [IRC today]https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2013-01-31&sort=asc#m544868 that would be a nice addition: Showing a preview in the modal if an oembed url is added.

comment:3 @helen3 years ago

  • Milestone changed from Future Release to 3.6

On the "Insert from URL" panel in the media modal:

nacin: so, A) detect oEmbed provider URLs, B) showing a preview of said video in the process, C) ensuring it gets inserted on its own line (force a line break) would complete this feature.

comment:4 @jtsternberg3 years ago

And one more reference link: [pre 3.6 post format overhaul user-testing]http://make.wordpress.org/ui/2013/01/31/have-made-it-through-the-second-round-of/

comment:5 follow-up: @jtsternberg3 years ago

Should this same functionality apply to all supported and applicable oEmbed urls? i.e. Tweets, slideshare, polldaddy, etc. Or should this only apply to the traditional "media" types like video/audio?

comment:6 in reply to: ↑ 5 ; follow-ups: @DrewAPicture3 years ago

Replying to jtsternberg:

Should this same functionality apply to all supported and applicable oEmbed urls? i.e. Tweets, slideshare, polldaddy, etc. Or should this only apply to the traditional "media" types like video/audio?

I feel like it would be a nice-to-have but may fall outside the scope of the ticket. Setting it up for video-only at least initially would be a good test case.

comment:7 in reply to: ↑ 6 ; follow-up: @helen3 years ago

Replying to DrewAPicture:

I feel like it would be a nice-to-have but may fall outside the scope of the ticket.

Without actually functionally writing code, I can't promise this, but it seems to me that if we're already detecting if it's an oEmbed provider and retrieving the response, it doesn't matter whether it's video, audio, a tweet, or something else.

comment:8 @jtsternberg3 years ago

I implemented very similar functionality with Custom Metaboxes and Fields for WordPress and plan on using that as a base for incorporating to WP for this ticket, but if anyone wants to take a look at the code and take a stab at it before I get time, hopefully that will help point you in the right direction.

comment:9 in reply to: ↑ 6 @jtsternberg3 years ago

Replying to DrewAPicture:

Replying to jtsternberg:

Should this same functionality apply to all supported and applicable oEmbed urls? i.e. Tweets, slideshare, polldaddy, etc. Or should this only apply to the traditional "media" types like video/audio?

I feel like it would be a nice-to-have but may fall outside the scope of the ticket. Setting it up for video-only at least initially would be a good test case.

In the case of the CMB script, it really made no difference what the oembed content was, so I THINK we can do the same here.

@jtsternberg3 years ago

This successfully displays an oembed preview when an oembed url is detected. Still needs a proper nonce passed for the ajax action.

comment:10 in reply to: ↑ 7 @jtsternberg3 years ago

  • Keywords has-patch added; needs-patch removed

As Helen pointed out in IRC, I think the next item to tackle is to remove the auto-linking when adding to a post via the media window. (which keeps the oEmbed from working on the front-end)

@helen3 years ago

comment:11 @gcorne3 years ago

  • Cc gregory@… added

comment:12 @sabreuse3 years ago

.2 is working nicely for me at embedding/previewing from the modal.

Last edited 3 years ago by sabreuse (previous) (diff)

comment:13 @helen3 years ago

Two things I want to see, besides nonce and removing auto-linking when a valid response is returned:

  • Testing usage of the Ajax action outside of the modal (looks like it'll work just fine, but would still like a test).
  • Thoughts on how we can deal with caching the retrieved data to stay away from rate limit problems. Right now all oEmbed caches are dumped on pre_post_update and then re-requested, which already makes it extremely easy to hit, say, Twitter's rate limit of 150 per hour per IP, especially on shared hosting. May very well be outside the scope of this ticket itself, but would at least like to see what thoughts might be.

comment:14 @helen2 years ago

  • Summary changed from Better Remote Video Insert to Preview oEmbed results when using the media modal to insert from URL

comment:15 @DrewAPicture2 years ago

  • Cc xoodrew@… added

comment:16 @ryan2 years ago

  • Milestone changed from 3.6 to Future Release

comment:17 @taylorde2 years ago

  • Cc td@… added

comment:18 @helen20 months ago

#24762 was marked as a duplicate.

comment:19 @helen19 months ago

#22548 was marked as a duplicate.

comment:20 @ircbot19 months ago

This ticket was mentioned in IRC in #wordpress-ui by helen. View the logs.

comment:21 @ircbot18 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:22 @ircbot16 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

@wonderboymusic16 months ago

comment:23 @wonderboymusic16 months ago

  • Milestone changed from Future Release to 4.0

15490.3.diff refreshes the Backbone pieces and leans on [28358] for the auto-parsing of [embed] in TinyMCE

comment:24 @jtsternberg16 months ago

I'm thinking we should remove the width/height inputs from the 'embed-link-settings' template. It's not applicable to all oembed types and it's probably not worth adding an if/else/switch statement to determine if they should be conditionally added. Also those widths should default to the [$content_width](http://codex.wordpress.org/Content_Width) global.

Thoughts? I'll provide a diff in case we're in agreement.

@jtsternberg16 months ago

Removes width/height inputs, and improves the styling of the oEmbed items in responsive views.

comment:25 @jtsternberg16 months ago

Last diff also contains css improvements. I tested at a window width of 645px and tested these oEmbed endpoints:

  • flickr
  • videopress (videopress)
  • wordpress.tv
  • vimeo
  • slideshare
  • imgur
  • youtube
  • instagram
  • twitter

comment:26 @jtsternberg16 months ago

One other thing I forgot to mention re: 15490.4.diff. I removed the width/height inputs from the template for reasons stated above, and on top of those, there was nothing written to handle the data entered in the width/height inputs.

comment:27 @ircbot16 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:28 @ircbot16 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:29 @wonderboymusic16 months ago

  • Owner filosofo deleted
  • Status changed from new to assigned

@wonderboymusic16 months ago

comment:31 @wonderboymusic16 months ago

In 28581:

When adding a URL in the Insert from URL state in the media modal, attempt to show a preview of the content. Drop the unused width and height fields.

This will probably be iterated upon.

Props helen, jtsternberg, wonderboymusic.
See #15490.

comment:32 @SergeyBiryukov16 months ago

Is there a need for global $post in wp_ajax_send_link_to_editor()?

comment:33 @wonderboymusic16 months ago

$post is pre-existing. There are probably a bunch of places where it is imported and never used. I will scan.

comment:34 @wonderboymusic16 months ago

Yes, it is required. $post (the global) is set later on down the road, $wp_embed requires it. If it is removed, the preview is correct, but a link is inserted into the editor.

comment:35 @wonderboymusic15 months ago

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

We are showing preview now, there are other tickets that address specifics.

Note: See TracTickets for help on using tickets.