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

Media Summary: Setup postdata for obtaining the excerpt #9348

Merged
merged 4 commits into from
Apr 21, 2018

Conversation

kraftbj
Copy link
Contributor

@kraftbj kraftbj commented Apr 18, 2018

Fixes #8420
Fixes #9209
Complement of #8510
Supports #8156

In PHP 7.2, warnings are thrown, in the end, because we are attempting to apply get_the_excerpt filters without being within the Loop. While Core makes it seem like this should work, it doesn't. (see #8510 for PR that would require Core changes to work as I think it should).

In this PR, we override the $post global and setup the post data so the excerpt and underlying functions work as intended.

We're overriding the $post since this function does not presume to operate within the Loop, so we need to fully mock the Loop.

Testing instructions:

@kraftbj kraftbj added [Type] Bug When a feature is broken and / or not performing as intended General [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Apr 18, 2018
@kraftbj kraftbj added this to the 6.1 milestone Apr 18, 2018
@kraftbj kraftbj requested a review from a team as a code owner April 18, 2018 16:51
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 added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Apr 20, 2018
@oskosk
Copy link
Contributor

oskosk commented Apr 20, 2018

@kraftbj should we leave an inline note (/todo) specifying that this code could be removed if core fixes something ?

@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 20, 2018
@kraftbj
Copy link
Contributor Author

kraftbj commented Apr 20, 2018

@oskosk Good idea. Added via 467eb0e .

@kraftbj kraftbj added the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 20, 2018
@dereksmart dereksmart merged commit 0c86be0 into master Apr 21, 2018
@dereksmart dereksmart deleted the fix/7.2except branch April 21, 2018 15:46
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 21, 2018
oskosk added a commit that referenced this pull request Apr 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General [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.

4 participants