Open Graph: update fallback images to return better images.#7246
Merged
Open Graph: update fallback images to return better images.#7246
Conversation
In #6297, we added transients to cache some of the fallback images in jetpack_og_get_image(). However, transients can only be 45 chars maximum. To work around that limitation, we now create a 44-char string using md5, based on the image URL. Also added back changes to Site Icon brought in #6303, and accidentally reverted in #6297.
We're already getting the site icon from the options table anyway, performance should be similar.
Image URL should always be an URL, image sizes should be integers.
* Grab the largest version of the theme logo we have. Facebook likes big images. * Return width and height values matching the size of the image, not matching the minimum image size we need.
* We want the width and height OG image tags to match the actual width and height of the site icon. * We want the largest image we have for the Site Icon, it will look better on Facebook.
- We now need a bigger test image, as images smaller than 200 x 200 px will not be used in OG tags. - We now use `wp_get_attachment_image_src` to grab the bigger version of the image that we can find.
5606144 to
bcfa67a
Compare
Member
Author
|
I fixed the unit tests. This should now be ready to review. |
zinigor
approved these changes
Oct 5, 2017
Contributor
zinigor
left a comment
There was a problem hiding this comment.
Read the code, tested, tags work fine, LGTM!
oskosk
approved these changes
Oct 17, 2017
Contributor
oskosk
left a comment
There was a problem hiding this comment.
Tested and works great! LGTM
jeherve
added a commit
that referenced
this pull request
Nov 15, 2017
I added esc_url in #7246, but it seems to be causing encoding issues with images including special characters and being passed through Photon. Fixes 1305-gh-jpop-issues Fixes 1306-gh-jpop-issues
zinigor
pushed a commit
that referenced
this pull request
Nov 20, 2017
I added esc_url in #7246, but it seems to be causing encoding issues with images including special characters and being passed through Photon. Fixes 1305-gh-jpop-issues Fixes 1306-gh-jpop-issues
zinigor
pushed a commit
that referenced
this pull request
Nov 20, 2017
I added esc_url in #7246, but it seems to be causing encoding issues with images including special characters and being passed through Photon. Fixes 1305-gh-jpop-issues Fixes 1306-gh-jpop-issues
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Testing instructions:
Test on a site using Jetpack for its Open Graph Meta tags:
og:image,og:image:width, andog:image:heightare always correct on the home page:Proposed changelog entry for your changes: