WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 18 months ago

#31502 assigned enhancement

Replace embed URL with error message if oEmbed provider returns an error

Reported by: mbootsman Owned by: deremohan
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 2.9
Component: Embeds Keywords: good-first-bug has-patch needs-testing
Focuses: Cc:

Description

If a youtube url in a post does not work when video is removed, or any other error occurs, the url of the video is shown. I tink it's better for UX to show an error message.

Attachments (3)

31502.diff (1000 bytes) - added by GaVrA 2 years ago.
Screen Shot 2015-09-06 at 2.07.14 PM.png (124.1 KB) - added by GaVrA 2 years ago.
31502.patch (799 bytes) - added by deremohan 2 years ago.

Download all attachments as: .zip

Change History (23)

#1 @SergeyBiryukov
3 years ago

  • Component changed from General to Embeds

#2 follow-up: @istein
3 years ago

Hey, I think that this is a good idea too. I'll try some things over the next few days and post what I come up with for people to discuss.

#3 in reply to: ↑ 2 @mbootsman
3 years ago

Replying to istein:

Hey, I think that this is a good idea too. I'll try some things over the next few days and post what I come up with for people to discuss.

Cool! Maybe obsolete, but please use a filter for this, so ppl can change the message if they want to.

#4 follow-up: @iseulde
3 years ago

  • Keywords reporter-feedback added

Do you mean in the editor, or the front-end? Maybe related: #29841. Links won't be converted to anything unless there's something from the provider to show. IMO it's up to them to show a message. If they do, we will show it.

#5 in reply to: ↑ 4 ; follow-up: @mbootsman
3 years ago

Replying to iseulde:

Do you mean in the editor, or the front-end? Maybe related: #29841. Links won't be converted to anything unless there's something from the provider to show. IMO it's up to them to show a message. If they do, we will show it.

I mean in the front-end. In the editor a embed image is shown. I think that if you try to embed something that does not exist, WordPress should tell that something went wrong, and have this message be filterable.

#6 in reply to: ↑ 5 ; follow-up: @jesin
3 years ago

Replying to mbootsman:

I mean in the front-end. In the editor a embed image is shown. I think that if you try to embed something that does not exist, WordPress should tell that something went wrong, and have this message be filterable.

I believe the embed_maybe_make_link can be used for this. It is present in tags/4.1/src/wp-includes/class-wp-embed.php#L332.

#7 follow-up: @iseulde
3 years ago

In the editor a embed image is shown.

Really? Can you give us an example video?

#8 in reply to: ↑ 7 @mbootsman
3 years ago

Replying to iseulde:

In the editor a embed image is shown.

Really? Can you give us an example video?

My bad, at first the embed image is hown, but when an invalid link is used, the url is shown. When this url is changed to a valid video, the video is shown.

#9 in reply to: ↑ 6 @mbootsman
3 years ago

Replying to jesin:

Replying to mbootsman:

I mean in the front-end. In the editor a embed image is shown. I think that if you try to embed something that does not exist, WordPress should tell that something went wrong, and have this message be filterable.

I believe the embed_maybe_make_link can be used for this. It is present in tags/4.1/src/wp-includes/class-wp-embed.php#L332.

Hmm, I missed that filter, seems usable! Thanks.

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

#10 follow-up: @iseulde
3 years ago

Can you give us an example video, please?

#11 in reply to: ↑ 10 @jesin
3 years ago

Replying to iseulde:

Can you give us an example video, please?

Take any video URL and remove a character from it.

Valid video - https://www.youtube.com/watch?v=2mBG4vlhcCc

Video doesn't exist - https://www.youtube.com/watch?v=2mBG4vlhcC

#12 @johnbillion
2 years ago

  • Keywords needs-patch good-first-bug added; reporter-feedback removed
  • Version changed from 4.1.1 to 2.9

#13 @johnbillion
2 years ago

  • Summary changed from Replace Youtube embed URL with error message if no video found to Replace embed URL with error message if oEmbed provider returns an error

@GaVrA
2 years ago

#14 @GaVrA
2 years ago

I added 31502.diff patch that does the following:

  1. checks response.success and if that is true it shows embed html in response.data.body
  2. if response.success is false it runs this.renderFail()
  3. inside this.renderFail() I check for response.data.type and if it is equal to not-embeddable it shows an error and returns.

#15 @GaVrA
2 years ago

I'm not sure if this is enough, maybe input should be cleared?

$('#embed-url-field' ).val('');?

Also if embed fails, you can still press Insert into page button. I'm not sure how would you go about disabling that since I'm not all that familiar with backbone.

#16 follow-up: @jesin
2 years ago

Patch 31502.diff only checks when adding an embed URL. OP's problem is what happens when an existing video is removed by the video hosting service.

#17 in reply to: ↑ 16 @deremohan
2 years ago

Replying to jesin:

Patch 31502.diff only checks when adding an embed URL. OP's problem is what happens when an existing video is removed by the video hosting service.

Hi Jesin,
As looking ajax call inside ajax-actions.php file at line 2818:

if ( $url && ! $parsed ) {
 $parsed = $wp_embed->run_shortcode( $shortcode );
}

If existing video is removed from hosting, then video url is not get parsed which leads to 'not-embeddable' type error. i mean this case should be handle by WP_Embed class.

Also please have a look for patch which also handles "not-ssl" response from ajax.

@deremohan
2 years ago

#19 @swissspidy
2 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

#20 @DrewAPicture
18 months ago

  • Owner set to deremohan
  • Status changed from new to assigned

Assigning to mark the good-first-bug as "claimed".

Note: See TracTickets for help on using tickets.