-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Post Title: Fix empty heading element when post_title is empty but get_the_title returns markup #70417
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
Post Title: Fix empty heading element when post_title is empty but get_the_title returns markup #70417
Conversation
|
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 Unlinked AccountsThe 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. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @shubhtoy! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
SirLouen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A review before testing
| $title = get_the_title(); | ||
|
|
||
| if ( ! $title ) { | ||
| if ( ! $title || ! strip_tags( $title ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the second part is true, the first part is always going to be true.
And if the second part is false the first part is always going to be false
And get_the_title always returns a string
https://developer.wordpress.org/reference/functions/get_the_title/
So the first part can technically be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ntsekouras you can merge this:
shubhtoy#1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ntsekouras you can merge this:
Not sure I could do that since it's the @shubhtoy's fork (permissions). @SirLouen I guess you could open a new PR, close this one and probably add @shubhtoy in props. There wasn't a reply for months since your comment (Jul 30) and there might not be in the future too :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ntsekouras just FTR, can you go into that and see if you have the merge available?
Theoretically, if you have write permissions here, and the creator of the PR has enabled maintainers to edit, you should be able to do the merge since it's the specific branch..
Confirm if you don't have this option here shubhtoy#1, and I will create a new PR but it would be good to know it.
FWIW, the other day, one maintainer did exactly this in a core Git PR, and I wondered if it's universal or only available to certain maintainers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you go into that and see if you have the merge available?
Sorry for not clarifying yesterday. I commented after going there and seeing that I couldn't merge. After searching it seems that this needs to be true:
the creator of the PR has enabled maintainers to edit
and the creator probably hasn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ntsekouras ok, created new #73841
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the second part is
true, the first part is always going to betrue. And if the second part isfalsethe first part is always going to befalseAnd
get_the_titlealways returns a string https://developer.wordpress.org/reference/functions/get_the_title/So the first part can technically be removed.
When the first part is true in | | condition, the second part won't be evaluated. Which is handy when following in checks in condition can be expensive.
Here's an example - https://3v4l.org/mV5CS#vnull.
What?
Closes #56465
Fix empty heading element when
post_titleis empty butget_the_titlereturns markup by checking for text content after stripping HTML tags.Why?
The Post Title block was outputting empty heading elements when plugins or themes filter
the_titleto add markup around an empty post title. This creates invalid HTML with headings that contain no text content, which can cause accessibility issues and semantic problems.The specific issue occurred when:
the_titleto wrap the empty title with HTML markup (e.g.,<span class="p-name"></span>)<h2><span class="p-name"></span></h2>- an empty headingThis was particularly problematic with plugins like wp-uf2 (Microformats) that add semantic markup to post titles.
How?
Updated the condition in
render_block_core_post_title()to check not only if the title is empty, but also if the title has no text content after stripping HTML tags usingstrip_tags().The fix changes:
To:
This ensures that if the title contains only HTML markup without any actual text content, the heading element will not be rendered at all, maintaining the original intention of not outputting empty headings.
Testing Instructions
functions.phpor create a test plugin:Alternatively, test with the wp-uf2 (Microformats) plugin:
<h2><a href="..."><span class="p-name"></span></a></h2>is renderedScreenshots or screencast
Before
9a79-7b6a-4fc6-99f0-0d788ac74ed8.mp4
After
1811-acfd-44da-a816-9b72830bff88.mp4