Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Post Images: add new method to detect images from Gutenberg blocks #11000

Merged
merged 13 commits into from
Jan 3, 2019

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Dec 18, 2018

Fixes #10501

Changes proposed in this Pull Request:

  • This new method parses all HTML content using WP's parse_blocks.
  • We look for Core image blocks, Core Gallery blocks, and Tiled Gallery blocks.
  • We also remove any image blocks that do not include a post ID. This is because image blocks that do not have a post ID are currently inserted using the "from image URL" option in the image block picker. As such, all the info in that block is the image URL; we have no data about image size for those images.
  • This PR also includes tests for such content.

Testing instructions:

  • Start from a site using no other Open Graph / Sharing plugins
  • Activate Jetpack's sharing module.
  • Create a new post using the new block editor.
  • Do not specify any Featured image
  • Do not upload any image to that post.
  • Instead, insert a new image block and choose from an image you've already uploaded, for another post (this is so Jetpack_PostImages does not pick images from attachments to your post)
  • Publish your post.
  • View source when looking at the post.
  • You should see your image used in the Open Graph Meta tags.
  • Repeat with an image that is smaller than the requirements (200x200): it should not be picked.
  • Repeat by inserting an image from another site, e.g. https://jeremy.hu/wp-content/uploads/watson-5818.jpg: it should not be picked.
  • Repeat by inserting a gallery block instead.
  • Repeat by inserting a tiled gallery block instead (note that this will only work once Tiled galleries: Consistent layouts wp-calypso#29559 is brought to Jetpack).

Proposed changelog entry for your changes:

  • Images: ensure that images inserted with new block editor can be used in Open Graph Meta tags, Related Posts, and Publicized posts.

Fixes #10501

- This new method parses all HTML content using WP's parse_blocks.
- We only select Core image blocks.
- We also remove any image blocks that do not include a post ID. This is because image
blocks that do not have a post ID are currently inserted using the "from image URL"
option in the image block picker. As such, all the info in that block is the image URL;
we have no data about image size for those images.
@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Sharing Post sharing, sharing buttons [Feature] Publicize Now Jetpack Social, auto-sharing [Feature] Related Posts [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Dec 18, 2018
@jeherve jeherve added this to the 6.9 milestone Dec 18, 2018
@jeherve jeherve self-assigned this Dec 18, 2018
@jeherve jeherve requested a review from a team December 18, 2018 17:54
@matticbot
Copy link
Contributor

D22486-code. (newly created revision)

@jetpackbot
Copy link

jetpackbot commented Dec 18, 2018

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: January 10, 2019.
Scheduled code freeze: January 3, 2019

Generated by 🚫 dangerJS against be19400

jeherve and others added 6 commits December 19, 2018 10:21
- It should be able to return the post URL when possible.
- When it does not return any post content (like an empty string), it should be handled
properly.
- We should fetch the post URL and not the post title.
This resulted in warnings because an opengraph tag function has tried to access the 'src' attribute of an empty array. Now the block parser doesn't return an empty array in case the image data is not found or not good enough.
zinigor
zinigor previously approved these changes Dec 25, 2018
Copy link
Member

@zinigor zinigor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work as expected, I have added one fix to avoid warnings, but overall this should be good to go.

@zinigor zinigor added [Status] Ready to Merge Go ahead, you can push that green button! [Status] Needs Testing We need to add this change to the testing call for this month's release and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Dec 25, 2018
Copy link
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@oskosk oskosk merged commit d545d4f into master Jan 3, 2019
@oskosk oskosk deleted the add/image-block-detection branch January 3, 2019 16:51
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jan 3, 2019
jeherve added a commit that referenced this pull request Jan 3, 2019
jeherve added a commit that referenced this pull request Jan 3, 2019
* Add first version of the Changelog and testing list for 6.9

* Changelog: add #10710

* changelog: add #10538

* changelog: add #10741

* changelog: add #10749

* changelog: add #10664

* changelog: add #10224

* changelog: add #10788

* Changelog: add #10560

* Chanegelog: add #10812

* changelog: add #10556

* Changelog: add #10668

* Changelog: add #10846

* Changelog: add #10947

* Changelog: add #10962

* Changelog: add #10956

* Changelog: add #10940

* Changelog: add #10934

* Changelog: add #10912

* changelog: add #10866

* changelog: add #10924

* Changelog: add #10936

* Changelog: add #10833

* changelog: add #10867

* Changelog: add #10960

* Changelog: add #10888

* changelog: add #10840

* changelog: add #10972

* Changelog: add #10979

* changelog: add #10909

* Changelog: add #10958

* Changelog: add #10981

* Changelog: add #10564

* Changelog: add #10809

* Changelog: add #10982

* Changelog: add #10706

* Changelog: add #10978

* Changelog: add #10132

* Changelog: add #11022

* Changelog: add #11024

* Changelog: add #10875

* Changelog: add #11030

* Changelog: add #11053

* Changelog: add #10880

* Changelog: add #9359

* Changelog: add #11037

* Update block list

* Changelog: add #11060

* Changelog: add #10755

* changelog: add #11000

* Changelog: add #10786

* Changelog: add #10945

* Changelog: add #10597
@jeherve jeherve removed the [Status] Needs Testing We need to add this change to the testing call for this month's release label Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Publicize Now Jetpack Social, auto-sharing [Feature] Related Posts [Feature] Sharing Post sharing, sharing buttons [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gutenberg: og:image tag does not pick the image from blocks
5 participants