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

Related Posts: Don't enque scripts in embed view #15890

Merged
merged 2 commits into from
May 22, 2020

Conversation

hinaloe
Copy link
Contributor

@hinaloe hinaloe commented May 22, 2020

Changes proposed in this Pull Request:

  • Prevent embedded posts from wastedly trying to get the related posts API

When embeded the post with oEmbed, the request will failed because sandbox of iframe. And, the response will not used.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Testing instructions:

  • Activate Jetpack and Related posts module
  • embed some post to another post
  • Open "another" post in Google Chrome
    • Before: Error like net::ERR_FAILED is shown in console
      Screenshot_20200522_165004
      • or, You can see flying request to API like /post/embed/?relatedposts=1
    • After: There are no one and There is no change in the appearance of the embedding
      • parent page still shows related posts

Proposed changelog entry for your changes:

  • Related posts: prevent embedded posts from wastedly trying to get the related posts API

@jetpackbot
Copy link

jetpackbot commented May 22, 2020

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: June 2, 2020.
Scheduled code freeze: May 26, 2020

Generated by 🚫 dangerJS against e4d791d

@jeherve jeherve added [Feature] Related Posts [Type] Bug When a feature is broken and / or not performing as intended labels May 22, 2020
@jeherve jeherve added this to the 8.6 milestone May 22, 2020
@jeherve jeherve added the [Status] Needs Review To request a review from Crew. Label will be renamed soon. label May 22, 2020
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Nice catch, thanks for the contribution! This should be good to merge, I only have one minor remark.

modules/related-posts/jetpack-related-posts.php Outdated Show resolved Hide resolved
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels May 22, 2020
@jeherve jeherve self-assigned this May 22, 2020
@jeherve
Copy link
Member

jeherve commented May 22, 2020

Internal reference: r207890-wpcom

@hinaloe
Copy link
Contributor Author

hinaloe commented May 22, 2020

Oops, it was missing. Thank you.

@jeherve jeherve merged commit b2e0833 into Automattic:master May 22, 2020
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels May 22, 2020
@hinaloe hinaloe deleted the patch-1 branch May 22, 2020 10:12
jeherve added a commit that referenced this pull request May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Related Posts [Status] Tested on WP.com Touches WP.com Files [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.

None yet

4 participants