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

Latest Posts block: Check priority of excerpt_length filter #33027

Open
BinaryMoon opened this issue Jun 28, 2021 · 12 comments
Open

Latest Posts block: Check priority of excerpt_length filter #33027

BinaryMoon opened this issue Jun 28, 2021 · 12 comments
Labels
[Block] Latest Posts Affects the Latest Posts Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Needs Technical Feedback Needs testing from a developer perspective. [Type] Enhancement A suggestion for improvement.

Comments

@BinaryMoon
Copy link

BinaryMoon commented Jun 28, 2021

Hi - the excerpt length setting in the latest posts block does not get used in all themes. This seems to be a common problem in themes that change the default excerpt length.

This appears to be a because of the excerpt_length filter in the render method.

add_filter( 'excerpt_length', 'block_core_latest_posts_get_excerpt_length', 20 );

In the method the filter has a priority of 20, however the official WordPress docs suggest a priority of 999 which I have found referenced on a number of sites and used in many themes.
https://developer.wordpress.org/reference/hooks/excerpt_length/#more-information

I started updating my 20+ themes to use a lower priority but then realised that it's used in so many other themes that it should probably be changed in core.

You can see how many public themes use it here:
https://wpdirectory.net/search/01F9927FX53NQ7AA33DS32YK38

If you expand the top 20 results on this page you will see that about 50% use the 999 priority.

My suggestion would be to use 9999 since some people (including myself) may go even higher in child themes.

@skorasaurus skorasaurus added [Block] Latest Posts Affects the Latest Posts Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Needs Technical Feedback Needs testing from a developer perspective. labels Jun 28, 2021
@jordesign jordesign changed the title Latest Posts excerpt length not working Post Excerpt: Check priority of excerpt_length filter Sep 14, 2023
@jordesign jordesign added the [Type] Enhancement A suggestion for improvement. label Sep 14, 2023
@jordesign jordesign changed the title Post Excerpt: Check priority of excerpt_length filter Latest Posts block: Check priority of excerpt_length filter Sep 14, 2023
@ramonjd
Copy link
Member

ramonjd commented Oct 10, 2023

I also noticed this while testing WordPress/wordpress-develop#5441 (comment)

I've tested using 99 and it seems to work.

Before

Editor Frontend
Screenshot 2023-10-10 at 12 11 21 pm Screenshot 2023-10-10 at 12 11 11 pm

After

Editor Frontend
Screenshot 2023-10-10 at 12 19 45 pm Screenshot 2023-10-10 at 12 20 02 pm

@ramonjd
Copy link
Member

ramonjd commented Oct 10, 2023

Testing if I can override in my theme also works.

I'm setting the priority to 999 in the latest posts block:

add_filter( 'excerpt_length', 'block_core_latest_posts_get_excerpt_length', 999 );

Then I add a hook in a function.php file for my theme:

function block_core_latest_posts_get_excerpt_length_again_totally( $length ) {
	return 4;
}

add_filter( 'excerpt_length', 'block_core_latest_posts_get_excerpt_length_again_totally', 9999 );

In the editor and frontend, the excerpt is trimmed to 4 words

Screenshot 2023-10-10 at 12 45 09 pm

@tomjn
Copy link
Contributor

tomjn commented Oct 10, 2023

@ramonjd behaviour is different based on how the excerpt was acquired, what would be useful is a mapping of all the ways excerpts can be shown, and when/how/if they're truncated, making the point to distinguish between user defined excerpts and auto-generated excerpts. Perhaps a table?

@tomjn
Copy link
Contributor

tomjn commented Oct 10, 2023

- Truncated by default Respects excerpt_length Respects length block attribute
Excerpt block (auto)
Excerpt block (manual) ? ? ?
the_excerpt() (auto)
the_excerpt() (manual) ? ?

There's a few gaps off the top of my head I'm unsure of their nuances, and I'm sure there are use cases I did not mention in the table, but it's a start

@ramonjd
Copy link
Member

ramonjd commented Oct 11, 2023

Thanks a lot for this @tomjn!

I find most of my confusion is reduced by bumping the priority of the filter to add_filter( 'excerpt_length', 'block_core_latest_posts_get_excerpt_length', 999 );

The only discrepancy I find is in the editor, which fetches the rendered excerpt from the post object (post.excerpt.rendered). It performs a bit of faffery to remove the outer HTML tags, which is fine, but only updates the excerpt after each save.

The discrepancy is most obvious when moving from a low word count to a higher one. Because the excerpt with a lower word count has been saved to the database, the editor cannot render the excerpt preview with a higher word count because those words are not available. Only after save are my changes reflected.

2023-10-11.12.31.00.mp4

@tomjn
Copy link
Contributor

tomjn commented Oct 11, 2023

hmmm that's unusual and looks like a separate bug with the latest posts block, can you create an issue to track that? Note that this is a new place I did not expect so it does not appear in the table I posted earlier. Be sure to include the copy pasted blocks when you open the issue so that people can copy paste it into their editor to reproduce the problem.

@ramonjd
Copy link
Member

ramonjd commented Oct 12, 2023

It appears to be a side effect of bumping the priority of the excerpt_length filter in /latest-posts/index.php and not reproducible in trunk.

I think the post object (in the editor) is returning the 55 word limit by default, so I guess, with the priority at 20 it's not taking into account any subsequent hooks that are firing.

I'll keep an ear out on this issue and do a follow up issue if we come up with a fix.

@tomjn
Copy link
Contributor

tomjn commented Oct 12, 2023

I've looked at the code and managed to update the table further:

- Truncated by default Respects excerpt_length
the_excerpt() (auto)
the_excerpt() (manual)
the_excerpt_embed() (auto)
the_excerpt_embed() (manual)
the_excerpt_rss() (auto)
the_excerpt_rss() (manual)
get_the_excerpt() (auto)
get_the_excerpt() (manual)
Post Excerpt block (auto) ⚠️ when using the REST API or WPAdmin a filter forces this to 100
Post Excerpt block (manual)
latest posts block (auto) ⚠️ only on filters greater than 10 priority, otherwise it's the value of the excerptLength attribute
latest posts block (manual)

Note that it's expected that user generated excerpts aka manual are not influenced by the excerpt_length filter, only auto-generated excerpts.

It's as accurate as the current code in the WP and GB github that I can follow

In this case the latest posts block is a bit redundant considering we have query blocks, I'd recommend using that in conjunction with a post excerpt block. The global variable is a bit of a curveball but can be sidestepped with the query block.

This also means that expected behaviour can be returned to both the excerpt block and the latest posts block if your excerpt length filter has a priority of 11 or higher

@tomjn
Copy link
Contributor

tomjn commented Oct 12, 2023

This also reveals that greater compatibility might be had if this code:

https://github.com/WordPress/gutenberg/blob/97caf7bff302fc115569493786a20d948468d241/packages/block-library/src/post-excerpt/index.php#L23C1-L25C4

was changed so that instead of falling back to the hardcoded value of 55 it instead used the value being filtered:

	$filter_excerpt_length = function( $length ) use ( $attributes ) {
		return isset( $attributes['wordCount'] ) ? $attributes['wordCount'] : $length;
	};

It could even be questioned that ! empty might be more appropriate so that wordCount set to false or 0 would indicate the block has chosen not to set a specific word limit and instead defers to WordPress' default behaviour.

Either of these would bring the excerpt block much closer to the expectations of those coming from classic themes while preserving the functionality added to the excerpt block.

Importantly, the latest posts block does not use the same mechanism for setting the excerpt, and could have its global variable removed and replaced with the same filter used by the excerpt block. Resolving this issue as well.

Finally, does it make sense to keep the filters using the same priority in these blocks, or should they instead use a lower priority such as 1? I can see arguments for both sides, what do you think?

@carolinan
Copy link
Contributor

I am not working on the excerpt.
It is still my opinion that for the blocks, the user's setting in the block should be used, not the theme filter.

@tomjn
Copy link
Contributor

tomjn commented Oct 12, 2023

It is still my opinion that for the blocks, the user's setting in the block should be used, not the theme filter.

I agree, this is to cover the situation when the user has not made such a setting and no block attribute is set.

If I were to write a user story:

Scenario 1: Respecting the excerpt length attribute

GIVEN I have added a latest posts block
AND I have set the `excerptLength` attribute to 5
AND I have used `the_excerpt` filter to change the max length of auto-generated excerpts to 10 words
THEN I should see at most 5 words on auto-generated excerpts when viewing the block

Scenario 2: Handling missing attributes

GIVEN I have added a latest posts block
AND the `excerptLength` attribute has not been set
AND I have used `the_excerpt` filter to change the max length of auto-generated excerpts to 10 words
THEN I should see at most 10 words on auto-generated excerpts when viewing the block

Scenario 1 passes. Scenario 2 fails.

Instead in the latest posts block it is hardcoded to 55.

@carolinan
Copy link
Contributor

That is helpful, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Latest Posts Affects the Latest Posts Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Needs Technical Feedback Needs testing from a developer perspective. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants