WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#11980 closed defect (bug) (fixed)

oembed not respecting Media -> Maximum embed size -> height

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

Description

When adding an oauth provider that is not in the current whitelist, via the following code in functions.php

add_action('init', 'movieclips_embeds');

function movieclips_embeds() {
    wp_oembed_add_provider( '#http://(www\.)?movieclips.com/watch/[\w]+/[\w]+/#i', 'http://movieclips.com/services/oembed/' , true);
}

The Maximum embed size "height" paramter (set in settings->media) seems to be ignored, sending a value of 700 to the provider unless explicitly set in the embed short tag. Please see attached screenshots for demonstration of bug to aid in my explanation.

Attachments (7)

media-settings.png (44.5 KB) - added by philipsoutham 6 years ago.
Showing that width is set to 560, height set to 304 in settings->media
post-content.png (47.7 KB) - added by philipsoutham 6 years ago.
Showing methods of embedding media, without short tag, with short tag, and with short tag where width and height are explicitly set.
add_provider-fuction-call.png (8.9 KB) - added by philipsoutham 6 years ago.
Showing code to call function to add non-whitelisted oembed provider.
wp_embed_defaults-function.png (24.7 KB) - added by philipsoutham 6 years ago.
Shows where height is statically set to 700 in wp-includes/media.php -> wp_embed_defaults function
calls-made-to-oembed-provider.png (10.9 KB) - added by philipsoutham 6 years ago.
Showing movieclips oembed provider log where maxheight parameter is set to 700 for all requests except for the one where height is explicitly set in embed short tag.
oembed-post-result.jpg (194.2 KB) - added by philipsoutham 6 years ago.
Showing embedded media on page as a result of publishing methods shown in attachment "post-content.png"
11980.patch (664 bytes) - added by Viper007Bond 6 years ago.

Download all attachments as: .zip

Change History (14)

@philipsoutham6 years ago

Showing that width is set to 560, height set to 304 in settings->media

@philipsoutham6 years ago

Showing methods of embedding media, without short tag, with short tag, and with short tag where width and height are explicitly set.

@philipsoutham6 years ago

Showing code to call function to add non-whitelisted oembed provider.

@philipsoutham6 years ago

Shows where height is statically set to 700 in wp-includes/media.php -> wp_embed_defaults function

@philipsoutham6 years ago

Showing movieclips oembed provider log where maxheight parameter is set to 700 for all requests except for the one where height is explicitly set in embed short tag.

@philipsoutham6 years ago

Showing embedded media on page as a result of publishing methods shown in attachment "post-content.png"

comment:1 @philipsoutham6 years ago

Correction to original ticket entry; "oauth" shoud be "oembed". Too many o* protocols...

comment:2 @ryan6 years ago

  • Keywords Viper007Bond added

comment:3 @ryan6 years ago

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

comment:4 @ryan6 years ago

  • Component changed from Media to oEmbed

comment:5 @Viper007Bond6 years ago

  • Keywords has-patch added

Hmm, how'd I manage that one?

@Viper007Bond6 years ago

comment:6 @nacin6 years ago

  • Keywords commit added
  • Milestone changed from Unassigned to 3.0
  • Version changed from 2.9.1 to 2.9

IMO, should be considered for the 2.9 branch to repair expected functionality.

Summing up from IRC: While width is the typical limiting factor, if the oembed provider does not follow an aspect ratio then it would be apparent. Plus, the only thing worse than a core option is an option in core that doesn't work.

comment:7 @ryan6 years ago

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

(In [12875]) Respect maximum embed height. Props Viper007Bond. fixes #11980

Note: See TracTickets for help on using tickets.