Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/wp-includes/general-template.php
Original file line number Diff line number Diff line change
Expand Up @@ -1424,8 +1424,12 @@ function wp_title( $sep = '»', $display = true, $seplocation = '' ) {
$title = __( 'Page not found' );
}

if ( ! is_string( $title ) ) {
$title = '';
}

$prefix = '';
if ( ! empty( $title ) ) {
if ( '' !== $title ) {
$prefix = " $sep ";
}

Expand All @@ -1436,7 +1440,7 @@ function wp_title( $sep = '»', $display = true, $seplocation = '' ) {
*
* @param string[] $title_array Array of parts of the page title.
*/
$title_array = apply_filters( 'wp_title_parts', ! empty( $title ) ? explode( $t_sep, $title ) : array() );
$title_array = apply_filters( 'wp_title_parts', explode( $t_sep, $title ) );
Copy link
Author

Choose a reason for hiding this comment

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

While I often create untitled posts on local sites and I expect that sometimes they are appropriate on live sites too, an empty $title string is hopefully uncommon. Also, even an empty array would run through implode() later anyway.

If skipping explode() is important, however, I think that parentheses make the condition a little easier to read.

Suggested change
$title_array = apply_filters( 'wp_title_parts', explode( $t_sep, $title ) );
$title_array = apply_filters( 'wp_title_parts', ( '' !== $title ) ? explode( $t_sep, $title ) : array() );

Copy link
Member

Choose a reason for hiding this comment

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

Suggest moving the conditional logic before apply_filters and passing the variable for better readability.

Copy link
Author

Choose a reason for hiding this comment

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

This could be more readable:

	if ( ! is_string( $title ) ) {
		$title = '';
	}

	$prefix      = '';
	$title_array = array();
	if ( '' !== $title ) {
		$prefix      = " $sep ";
		$title_array = explode( $t_sep, $title );
	}

	/**
	 * Filters the parts of the page title.
	 *
	 * @since 4.0.0
	 *
	 * @param string[] $title_array Array of parts of the page title.
	 */
	$title_array = apply_filters( 'wp_title_parts', $title_array );

We probably should change the $sep and/or $t_sep names too, but not for this release.

Copy link
Author

Choose a reason for hiding this comment

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

If the apply_filters( 'wp_title_parts', $title_array ) direction above is worth pursuing, it might be better in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there's any need to go to a new PR for that change. It looks good to me.

Copy link
Author

Choose a reason for hiding this comment

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

I switched this PR to the more readable concept and checked the following URLs with the updated .diff and Twenty Fourteen.

  • http://localhost/svn/src/0-2/ (with 0 as the post title): <title>0 | Trunk (SVN)</title>
  • http://localhost/svn/src/ (front page, without a static front page): <title>Trunk (SVN)</title>
  • http://localhost/svn/src/sample-page/ (page of posts): <title>Sample Page | Trunk (SVN)</title>

Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicated the same tests with Twenty Twenty, and confirmed same behavior.


// Determines position of the separator and direction of the breadcrumb.
if ( 'right' === $seplocation ) { // Separator on right, so reverse the order.
Expand Down
Loading