Skip to content

Conversation

@SirLouen
Copy link
Member

@SirLouen SirLouen commented Dec 9, 2025

Fixes #56465

This is a continuation of #70417, here you can find full context
Just for some context:

Fix empty heading element when post_title is empty but get_the_title returns markup by checking for text content after stripping HTML tags.

cc @ntsekouras

@github-actions
Copy link

github-actions bot commented Dec 9, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @[email protected].

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: [email protected].

Co-authored-by: SirLouen <[email protected]>
Co-authored-by: ntsekouras <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: inc2734 <[email protected]>
Co-authored-by: shubhtoy <[email protected]>
Co-authored-by: jessedyck <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@SirLouen SirLouen requested review from ntsekouras and removed request for ajitbohra and fabiankaegy December 9, 2025 09:57
@SirLouen SirLouen self-assigned this Dec 9, 2025
@SirLouen SirLouen added [Type] Bug An existing feature does not function as intended [Block] Post Title Affects the Post Title Block labels Dec 9, 2025
Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Thank you!

$title = get_the_title();

if ( ! $title ) {
if ( ! strip_tags( $title ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

Left a note regarding using both (! title || ! strip_tags()) checks in the original PR - #70417 (comment).

Copy link
Member Author

@SirLouen SirLouen Dec 9, 2025

Choose a reason for hiding this comment

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

@Mamaduka technically that is correct.

But this evaluation interruption is only useful when there is actually a complex task behind the second.

Doing a strip_tags of an empty string is barely a function initialization, almost the same time complexity as the first comparison in terms of performance.

So I feel it's unnecessarily redundant, but as you say, technically it's more efficient, so I would go with whatever you prefer.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, it depends on the case. I don't have a strong opinion here; either option is fine with me.

Copy link
Member Author

@SirLouen SirLouen Dec 9, 2025

Choose a reason for hiding this comment

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

I've been profiling, and with a couple million iterations it scales faster without the first check.

Copy link
Member Author

@SirLouen SirLouen Dec 9, 2025

Choose a reason for hiding this comment

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

https://github.com/php/php-src/blob/c380f7e38298f479342397ac0bcfd0bf2bad2f86/ext/standard/string.c#L4820-L4861

The problem is that with an empty string, still gets to php_strip_tags_ex and allocates a buffer with the duplicated empty string. Over 10 million iterations, there is almost a 3x difference in total. It doesn't scale too crazily but is something to be considered.

I've reported this upstream, reporting a possible PR to improve this a little bit.
php/php-src#20675

It's not going to come anytime early, but it could help us leave this simpler.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you, @SirLouen!

@Mamaduka Mamaduka enabled auto-merge (squash) December 9, 2025 18:24
@Mamaduka Mamaduka merged commit feeceda into WordPress:trunk Dec 9, 2025
80 of 87 checks passed
@github-actions github-actions bot added this to the Gutenberg 22.3 milestone Dec 9, 2025
@SirLouen SirLouen deleted the patch/56465 branch December 9, 2025 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Block] Post Title Affects the Post Title Block [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Post Title: Empty heading element when post_title is empty but get_the_title returns markup

3 participants