Make WordPress Core

Opened 4 years ago

Last modified 3 years ago

#52820 reviewing defect (bug)

Possible logic error in wp_trim_excerpt()

Reported by: themiked's profile theMikeD Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Posts, Post Types Keywords: has-patch has-unit-tests reporter-feedback
Focuses: Cc:

Description

https://core.trac.wordpress.org/browser/tags/5.7/src/wp-includes/formatting.php#L3845

Reading this code, it looks like the wp_trim_excerpt filter's $raw_excerpt parameter will be empty if $text as supplied to the function is empty. This should be the value of $text at L3824 should it not? Or do I mis-understand?

Change History (11)

This ticket was mentioned in PR #1126 on WordPress/wordpress-develop by donmhico.


4 years ago
#1

  • Keywords has-patch has-unit-tests added

This PR assigns the correct value for $raw_excerpt if the passed $text is empty. Right now $raw_excerpt is empty if $text is empty.

Trac ticket: https://core.trac.wordpress.org/ticket/52820

#2 @donmhico
4 years ago

Hello @theMikeD. Thanks for the ticket. Yes, I believe you're correct. $raw_excerpt passed to wp_trim_excerpt filter is empty if $text is empty. In my PR above, I placed $raw_excerpt = $text; below the apply_filter( 'the_content' ) because I still believe that $raw_excerpt should be processed by that filter. Others can chime in their thoughts regarding this.

#3 @SergeyBiryukov
4 years ago

  • Component changed from General to Posts, Post Types
  • Milestone changed from Awaiting Review to 5.8
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#4 @hellofromTonya
4 years ago

  • Milestone changed from 5.8 to 5.9

Today is 5.8 Beta 1. Ran out of time for this one to land. Punting to 5.9.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 years ago

#6 @audrasjb
3 years ago

  • Milestone changed from 5.9 to 6.0

We have some conflicts to resolve in the above PR. As today is 5.9 beta 1, I think it's safer to move this ticket to 6.0 to ensure it's properly reviewed and fixed.

This ticket was mentioned in Slack in #core by mike. View the logs.


3 years ago

#9 in reply to: ↑ description @jrf
3 years ago

Replying to theMikeD:

https://core.trac.wordpress.org/browser/tags/5.7/src/wp-includes/formatting.php#L3845

Reading this code, it looks like the wp_trim_excerpt filter's $raw_excerpt parameter will be empty if $text as supplied to the function is empty. This should be the value of $text at L3824 should it not? Or do I mis-understand?

I do agree there may be a logic error in this function, but I don't necessarily think it's in the filter. Maybe we should have good look at what this function is supposed to do ?

From the docblock:

3798 	 * Generates an excerpt from the content, if needed.
3799	 *
3800	 * Returns a maximum of 55 words with an ellipsis appended if necessary.
...
3808	 * @param string             $text Optional. The excerpt. If set to empty, an excerpt is generated.
3809	 * @param WP_Post|object|int $post Optional. WP_Post instance or Post ID/object. Default null.
3810	 * @return string The excerpt.

Looking at the code, the excerpt is only trimmed to 55 words when it has been auto-generated, not when it was provided via the $text parameter.

Is this really the intended behaviour ?

#10 @costdev
3 years ago

  • Keywords reporter-feedback added

As this is still being discussed and we're approaching RC1, I'm going to move this ticket to Future Release.

#11 @costdev
3 years ago

  • Milestone changed from 6.0 to Future Release
Note: See TracTickets for help on using tickets.