Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#34879 closed defect (bug) (fixed)

Typo in wp_title separator placeholder

Reported by: morganestes's profile morganestes Owned by: swissspidy's profile swissspidy
Milestone: 4.5 Priority: lowest
Severity: trivial Version: 1.0
Component: General Keywords: good-first-bug has-patch has-unit-tests
Focuses: template Cc:

Description (last modified by morganestes)

The placeholder separator within wp_title() is %WP_TITILE_SEP%. A contributor to the Code Reference noticed the typo and wanted to confirm if it was an issue with the code itself or the reference page, so I'm confirming here it's in the code and wonder if it needs to be fixed since it's now in the reference.

See https://core.trac.wordpress.org/browser/trunk/src/wp-includes/general-template.php#L998 for the details.

Attachments (2)

34879.patch (653 bytes) - added by joelerr 10 years ago.
34879-2.patch (1.5 KB) - added by dossy 10 years ago.

Download all attachments as: .zip

Change History (18)

#1 @morganestes
10 years ago

  • Description modified (diff)

@joelerr
10 years ago

#2 @danhgilmore
10 years ago

  • Keywords has-patch added

#3 @SergeyBiryukov
10 years ago

  • Milestone changed from Awaiting Review to 4.5

#4 follow-up: @DrewAPicture
10 years ago

  • Keywords needs-unit-tests added

For reference, this typo has been in place for since [9376] (7 years). Kind of makes you wonder why it was never previously caught. I think we could benefit from unit tests here.

Version 0, edited 10 years ago by DrewAPicture (next)

#5 @morganestes
10 years ago

  • Owner set to morganestes
  • Status changed from new to assigned

#6 follow-up: @morganestes
10 years ago

I suspect it was never caught because it's only used by that one function. I'll check the theme and plugin repos to see if I can find any reference to it, but suspect I'll come up empty.

What kind of tests can we write for it?

UPDATE: Although it's not filterable, it's passed to 'wp_title_parts', so that's where I'm going to start looking.

Last edited 10 years ago by morganestes (previous) (diff)

#7 in reply to: ↑ 4 @SergeyBiryukov
10 years ago

Replying to DrewAPicture:

For reference, this typo has been in place since [9376] (7 years). Kind of makes you wonder why it was never previously caught.

FWIW, I've seen it before, but thought it was some kind of a legacy thing :)

#8 follow-up: @nacin
10 years ago

This is amusing.

#9 in reply to: ↑ 8 @morganestes
10 years ago

Replying to nacin:

This is amusing.

It's a minor issue, but it was noticed and questioned when someone caught it while using the Developer Resources site.
I figure that's a win for DevHub, plus a good chance to learn about testing legacy code. Sure, it may not be high priority for fixing, but it's a great learning opportunity for new contributors.

#10 in reply to: ↑ 6 @morganestes
10 years ago

Replying to morganestes:

I suspect it was never caught because it's only used by that one function. I'll check the theme and plugin repos to see if I can find any reference to it, but suspect I'll come up empty.

UPDATE: Although it's not filterable, it's passed to 'wp_title_parts', so that's where I'm going to start looking.

Ran through the theme and plugin repos; the filter's not used in any theme, but it's used in 6 plugins, although the separator isn't specifically modified in any of them.

@dossy
10 years ago

#11 @dossy
10 years ago

The $t_sep value shouldn't really be testable since it's an internal implementation detail of the function, not meant to be exposed to any caller. However, if you really want tests that fail given the current typo, I can write one - although, it'll be very tightly coupled to the specific implementation of wp_title() and not really be testing the higher-level functionality (e.g., if wp_title() is refactored, the test will fail but the function could still likely be doing the right thing in general).

Really, shouldn't wp_title()'s implementation get rewritten to use the new wp_get_document_title() in 4.4?

Anyway, I've uploaded 34879-2.patch which adds a test that will fail without the "fix" if that helps.

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


10 years ago

#13 @swissspidy
10 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

#14 @mrahmadawais
10 years ago

Good catch @morganestes :)

#15 @DrewAPicture
10 years ago

  • Owner changed from morganestes to swissspidy

#16 @ocean90
10 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 36540:

In wp_title() fix a 7 year old typo.

Props joelerr.
Fixes #34879.

Note: See TracTickets for help on using tickets.