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 on AMP: Lower-priority filter hooks are removed #13775

Closed
micropat opened this issue Oct 18, 2019 · 2 comments · Fixed by #13804
Closed

Related Posts on AMP: Lower-priority filter hooks are removed #13775

micropat opened this issue Oct 18, 2019 · 2 comments · Fixed by #13804
Assignees
Labels
AMP [Feature] Related Posts [Type] Bug When a feature is broken and / or not performing as intended
Milestone

Comments

@micropat
Copy link

micropat commented Oct 18, 2019

Filter hooks with a low-priority number of 41 or greater are removed on AMP pages when the Related Posts module is activated and setup.

Steps to reproduce the issue

  1. Add the PHP code below to the site's functions.php or similar.
  2. Activate & setup the official AMP plugin.
  3. Observe that additional content is added to post content on an AMP page.
  4. Activate & setup Jetpack's Related Posts module.
  5. Observe that additional content is not added to post content on an AMP page.
add_filter( 'the_content', 'add_something_to_content_filter', 41 );

function add_something_to_content_filter( $content ) {
	$original_content   = $content;
	$add_before_content = 'SOMETHING BEFORE CONTENT.';
	$add_after_content  = 'SOMETHING AFTER CONTENT.';
	
	$content = $add_before_content . $original_content . $add_after_content;

	return $content;
}

What I expected

SOMETHING BEFORE CONTENT. should prepend content.
SOMETHING AFTER CONTENT. should append content.

What happened instead

No additional content is added to content.

@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended AMP [Feature] Related Posts labels Oct 18, 2019
@jeherve
Copy link
Member

jeherve commented Oct 18, 2019

We've been looking at this as well, most likely thanks to a customer we have in common. :)

Internal reference: 4412-gh-jpop-issues

@kbrown9 has been spearheading this investigation so far, and here is what she's found so far:

This issue is caused by a bug in WordPress that was first reported ~10 years ago. Multiple tickets have been opened for this over the years, but a fix hasn't been accepted.

https://core.trac.wordpress.org/ticket/9968
https://core.trac.wordpress.org/ticket/40393
https://core.trac.wordpress.org/ticket/40685

Until this gets fixed in Core, I'll try to figure out a way to avoid this bug in the Related Posts feature.

kbrown9 added a commit that referenced this issue Oct 22, 2019
Fixes #13775

The filter 'filter_add_target_to_dom()' removes itself from the 'the_content'
hook. Then, a bug in Core causes the filter with the next highest priority to
be skipped.

For more information on the Core bug see: https://core.trac.wordpress.org/ticket/9968

To avoid this bug, remove the call to "remove_filter( 'the_content', array( $this,
'filter_add_target_to_dom'), ...)". Instead, just rely on conditionals in the
'filter_add_target_to_dom()' method to control when related posts are added to the
content.

Related posts should not be added to the content under these three condtions:
1. The post contains a Related Posts block.
2. The post contains a 'jetpack-related-posts' shortcode.
3. The 'get_the_excerpt' hook is being executed. This hook is executed when the related
   posts are being generated, and related posts should not be added to the related post
   content.

These conditions are already handled by the 'filter_add_target_to_dom()' method. However,
legacy related posts are currently added to posts that contain a Related Posts block. Fix this
by removing the $content parameter from "has_block('jetpack/related-posts', $content)".
@kbrown9
Copy link
Member

kbrown9 commented Oct 22, 2019

Thanks for the bug report @micropat!

I just want to add an additional detail about this bug. If there are multiple filters with priorities greater than 40, only the first filter with a priority greater than 40 is skipped. All of the other filters are applied.

For example, if you try this code, you’ll see that 'add_something_to_content_filter2’ is applied.

add_filter( 'the_content', 'add_something_to_content_filter', 41 );

function add_something_to_content_filter( $content ) {
	$original_content   = $content;
	$add_before_content = 'SOMETHING BEFORE CONTENT.';
	$add_after_content  = 'SOMETHING AFTER CONTENT.';
	
	$content = $add_before_content . $original_content . $add_after_content;

	return $content;
}

add_filter( 'the_content', 'add_something_to_content_filter2', 42 );

function add_something_to_content_filter2( $content ) {
	$original_content   = $content;
	$add_before_content = 'SOMETHING BEFORE CONTENT 2 .';
	$add_after_content  = 'SOMETHING AFTER CONTENT 2 .';
	
	$content = $add_before_content . $original_content . $add_after_content;

	return $content;
}

@jeherve jeherve added this to the 7.9 milestone Oct 23, 2019
jeherve pushed a commit that referenced this issue Oct 23, 2019
Fixes #13775

The filter 'filter_add_target_to_dom()' removes itself from the 'the_content'
hook. Then, a bug in Core causes the filter with the next highest priority to
be skipped.

For more information on the Core bug see: https://core.trac.wordpress.org/ticket/9968

To avoid this bug, remove the call to "remove_filter( 'the_content', array( $this,
'filter_add_target_to_dom'), ...)". Instead, just rely on conditionals in the
'filter_add_target_to_dom()' method to control when related posts are added to the
content.

Related posts should not be added to the content under these three condtions:
1. The post contains a Related Posts block.
2. The post contains a 'jetpack-related-posts' shortcode.
3. The 'get_the_excerpt' hook is being executed. This hook is executed when the related
   posts are being generated, and related posts should not be added to the related post
   content.

These conditions are already handled by the 'filter_add_target_to_dom()' method. However,
legacy related posts are currently added to posts that contain a Related Posts block. Fix this
by removing the $content parameter from "has_block('jetpack/related-posts', $content)".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AMP [Feature] Related Posts [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants