Make WordPress Core

Opened 7 weeks ago

Last modified 20 hours ago

#62094 reviewing defect (bug)

/wp-json/oembed/1.0/embed returns too big thumbnail_url

Reported by: colinleroy's profile colinleroy Owned by: swissspidy's profile swissspidy
Milestone: 6.8 Priority: normal
Severity: normal Version: 4.4
Component: Embeds Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

function get_oembed_response_data_rich( $data, $post, $width, $height )
uses $min_max_width (default 200-600),
and if that size of thumbnail is not generated, the full-scale url is returned in thumbnail_url field (leading to too big files being sent).

Attachments (2)

wp-62094.patch (690 bytes) - added by colinleroy 7 weeks ago.
This patch is a quick fix that uses the 'medium_large' size
wp-62094-2.patch (694 bytes) - added by colinleroy 7 weeks ago.
Correct patch. The constrained 99999 height was messing the ratio calculation, preventing image_get_intermediate_size() to consider good candidates.

Download all attachments as: .zip

Change History (12)

@colinleroy
7 weeks ago

This patch is a quick fix that uses the 'medium_large' size

@colinleroy
7 weeks ago

Correct patch. The constrained 99999 height was messing the ratio calculation, preventing image_get_intermediate_size() to consider good candidates.

#1 @colinleroy
7 weeks ago

My second patch fixes the problem cleanly for me. I hope it is correct :)

This ticket was mentioned in PR #7422 on WordPress/wordpress-develop by colinleroy.


7 weeks ago
#2

  • Keywords has-patch added

Use 0 as height, so that the closest bigger available image size will be used as thumbnail_url, instead of 99999 which prevented scaling down from happening at all.

Trac ticket: https://core.trac.wordpress.org/ticket/62094

#3 @colinleroy
6 weeks ago

For now I'm working around that with a filter in functions.php, but that is suboptimal:

function fix_image_downsize_99999($downsize, $id, $size) { 
  if (is_array($size) && isset($size[1]) && $size[1] == 99999) {
    $size[1] = 0;
    return image_downsize($id, $size);
  }
  return $downsize;
}
add_filter( "image_downsize", "fix_image_downsize_99999", 10, 3 );

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


6 weeks ago

#5 @swissspidy
6 weeks ago

  • Keywords needs-unit-tests needs-testing-info added
  • Version set to 4.4

Hi @colinleroy and thanks for your contribution!

Would you be willing to write some unit tests for this change to verify that this does what it is supposed to do? Alternatively, some manual testing instructions would be very helpful, so that this could be verified by hand, or used to write tests.

PS. No need to add both a patch file and a PR. A PR alone is enough :-)

#7 @colinleroy
6 weeks ago

Thank you for your feedback!

I've updated the PR with a unit test. I hope it's what you expected.

For a manual reproduction, create a post or page; add a large image in the post, and set it as Featured Image. Publish the post, and check its <link rel="alternate" title="oEmbed (JSON)" type="application/json+oembed" href="...">

With this patch, it'll look like:

{
  "version": "1.0",
  "provider_name": "colin@colino.net",
  "provider_url": "https://www.colino.net/wordpress",
  "author_name": "Colin",
  "author_url": "https://www.colino.net/wordpress/archives/author/colin/",
  "title": "F*** capitalism in general and f*** Youtube in particular",
  "type": "link",
  "width": 600,
  "height": 338,
  "thumbnail_url": "https://www.colino.net/wordpress/wp-content/uploads/image-97-768x576.png",
  "thumbnail_width": 600,
  "thumbnail_height": 450
}

Without the patch,

{
  "version": "1.0",
  "provider_name": "colin@colino.net",
  "provider_url": "https://www.colino.net/wordpress",
  "author_name": "Colin",
  "author_url": "https://www.colino.net/wordpress/archives/author/colin/",
  "title": "F*** capitalism in general and f*** Youtube in particular",
  "type": "link",
  "width": 600,
  "height": 338,
  "thumbnail_url": "https://www.colino.net/wordpress/wp-content/uploads/image-97.png",
  "thumbnail_width": 600,
  "thumbnail_height": 450
}

In the second case, the served thumbnail_url is the full-size image.

Version 0, edited 6 weeks ago by colinleroy (next)

#8 @swissspidy
5 weeks ago

  • Keywords has-unit-tests added; needs-unit-tests needs-testing-info removed
  • Milestone changed from Awaiting Review to 6.8
  • Owner set to swissspidy
  • Status changed from new to reviewing

@colinleroy commented on PR #7422:


21 hours ago
#9

Hello @swissspidy !
I've seen 6.7 has shipped, do you want to merge this one? :)
Thanks!

@swissspidy commented on PR #7422:


20 hours ago
#10

It‘s on my todo list for after my vacation. Not in a hurry :)

Note: See TracTickets for help on using tickets.