WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#11964 closed defect (bug) (fixed)

WP_oEmbed::fetch() doesn't give any results if the provider doesn't support JSON

Reported by: nbachiyski Owned by: Viper007Bond
Milestone: 3.0 Priority: normal
Severity: normal Version:
Component: Embeds Keywords: has-patch commit
Focuses: Cc:

Description

Currently:

A request is made for JSON format and if we get an error we give up on that provider.

Expected:

According to the oEmbed specification a provider should return 501 Not Implemented if it doesn't support the format. We should respect that and try the next format, instead of immediately failing.

Attachments (1)

oembed-respect-not-implemented.diff (2.8 KB) - added by nbachiyski 4 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 scribu4 years ago

  • Milestone changed from Unassigned to 3.0

comment:2 ryan4 years ago

  • Component changed from Media to oEmbed

comment:3 nacin4 years ago

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

This looks good, working fine here. Maybe fetch_with_format() should be private as well as _parse_json and _parse_xml?

Assigning to Viper007Bond for a review.

comment:4 Viper007Bond4 years ago

  • Keywords commit added

I chatted privately with Nikolay back when he wrote this patch and it looked good to me then. It should be re-tested now and if it works, committed.

And yes, I agree fetch_with_format() should be private as well.

comment:5 nacin4 years ago

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

(In [13996]) Request XML from an oEmbed provider if the provider returns '501 Not Implemented' for JSON. props nbachiyski, fixes #11964.

comment:6 nbachiyski4 years ago

What is the point of inline docs, which say exactly the same as the function definition:

Parses a json response body.

vs.

function _parse_xml( $response_body )

I like inline docs when they add value, not when they just add clutter.

Does phpDocumentator show methods starting with underscore, or we have to explicitly set the @private directive?

comment:7 nacin4 years ago

Yeah, you have to explicitly set @access private, and I don't like adding functions without @since. The rest I wrote without even thinking about it, though I don't think it's a complete waste of value. Even though it isn't included in phpDocumentor (unless there is a flag set), it also helps with phpxref.

comment:8 nbachiyski4 years ago

@since and @access are fine by me. My main point was that description doesn't add any value, because it just repeats the method name and the argument list.

Anyway, this is not the right place for further discussion.

Note: See TracTickets for help on using tickets.