WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 3 months ago

#41299 closed defect (bug) (fixed)

oEmbed proxy fails to forward maxwidth and maxheight params

Reported by: westonruter Owned by: westonruter
Milestone: 4.8.1 Priority: normal
Severity: normal Version: 4.8
Component: Embeds Keywords: has-patch has-unit-tests commit fixed-major
Focuses: rest-api Cc:

Description

oEmbed uses the maxwidth and maxheight params, and the oEmbed proxy endpoint added in core as of #40450 uses the same. However, it turns out that that WP_oEmbed::fetch() uses width and height instead. So currently passing these maxwidth and maxheight params have no effect. This is causing a problem in Gutenberg where the oEmbed needs to be written into a sandbox iframe that has specific dimensions: https://github.com/WordPress/gutenberg/pull/1688#issuecomment-314862861

Attachments (2)

41299.0.diff (4.8 KB) - added by westonruter 3 months ago.
https://github.com/xwp/wordpress-develop/pull/239
41299.1.diff (5.1 KB) - added by westonruter 3 months ago.
Refresh patch after committing fix for #41048

Download all attachments as: .zip

Change History (15)

#1 @westonruter
3 months ago

  • Keywords has-patch has-unit-tests added

#2 @westonruter
3 months ago

  • Owner set to jnylen0
  • Status changed from new to reviewing

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


3 months ago

#4 @swissspidy
3 months ago

Shouldn't the unset( $args['_wpnonce'] ); part be handled by #41048? Or do you want to close that one as a duplicate?

#5 @westonruter
3 months ago

@swissspidy Yes, it can be committed as part of #41048. I wasn't aware of that other ticket when I was making this patch, so I was just going to sneak it in.

@westonruter
3 months ago

Refresh patch after committing fix for #41048

#6 @westonruter
3 months ago

  • Keywords commit added

@swissspidy does 41299.1.diff look good to you?

Build passes: https://travis-ci.org/xwp/wordpress-develop/builds/253005279

#7 @swissspidy
3 months ago

@westonruter works great!

#8 follow-up: @jnylen0
3 months ago

Sorry, I just saw that you pinged me for review here. Code looks good and fixes the related Gutenberg issue mentioned in the OP. I tested as follows:

  • Added the ?maxwidth= parameter in the Gutenberg code
  • Loaded various embeds with and without this patch
  • Before loading each embed, clear the cached results by running foreach ( $wpdb->get_results("SELECT option_name FROM $wpdb->options WHERE option_name LIKE '_transient%oembed%'") as $row ) { error_log( $row->option_name ); delete_option( $row->option_name ); } in wp shell

+1 for commit.

Last edited 3 months ago by jnylen0 (previous) (diff)

#9 @westonruter
3 months ago

  • Owner changed from jnylen0 to westonruter
  • Status changed from reviewing to accepted

#10 in reply to: ↑ 8 @westonruter
3 months ago

Replying to jnylen0:

  • Before loading each embed, clear the cached results by running foreach ( $wpdb->get_results("SELECT option_name FROM $wpdb->options WHERE option_name LIKE '_transient%oembed%'") as $row ) { error_log( $row->option_name ); delete_option( $row->option_name ); } in wp shell

BTW, you can also use wp transient delete --all.

#11 @westonruter
3 months ago

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

In 41047:

REST API: Ensure maxwidth and maxheight params are forwarded to oEmbed provider in proxy requests.

Also correct phpdoc return tag on WP_oEmbed_Controller::get_proxy_item() and remove dead code in oEmbed controller phpunit tests.

Amends [40628].
See #40450.
Fixes #41299.

#12 @westonruter
3 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening for 4.8.1

#13 @westonruter
3 months ago

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

In 41049:

REST API: Ensure maxwidth and maxheight params are forwarded to oEmbed provider in proxy requests.

Also correct phpdoc return tag on WP_oEmbed_Controller::get_proxy_item() and remove dead code in oEmbed controller phpunit tests.

Merges [41047] into 4.8 branch.
Amends [40628].
See #40450.
Fixes #41299 for 4.8.1.

Note: See TracTickets for help on using tickets.