Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#51643 closed enhancement (fixed)

Function capital_P_dangit is applied directly to document title

Reported by: dragunoff's profile dragunoff Owned by: desrosj's profile desrosj
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Formatting Keywords: good-first-bug has-patch needs-dev-note
Focuses: Cc:

Description

This formatting function is normally applied via filters. But in wp_get_document_title it's called directly and thus its effect is forced onto the user. It should instead be hooked to the filter document_title_parts.

Attachments (2)

51643.diff (1.8 KB) - added by paaggeli 3 years ago.
Create a new filter 'document_title' and hooked the wptexturize(), convert_chars(), capital_P_dangit().
51643.1.diff (1.8 KB) - added by audrasjb 3 years ago.

Download all attachments as: .zip

Change History (12)

#1 @jeremyfelt
3 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect (bug) to enhancement

It does seem odd that wptexturize(), convert_chars(), and capital_P_dangit() are all run on the title after the document_title_parts filter is applied in wp_document_title().

I'm not sure if we can adjust the order now that it's been there for a few years, but it looks like there's room for a document_title filter that is applied before wp_document_title() returns $title. We can then use the familiar pattern (general-template.php) to apply each of those in a way that can be unhooked.

Because wp_document_title() is expected to return a string safe for use in <title></title>, esc_html() should probably be run after the document_title filter is applied.

Version 0, edited 3 years ago by jeremyfelt (next)

@paaggeli
3 years ago

Create a new filter 'document_title' and hooked the wptexturize(), convert_chars(), capital_P_dangit().

#2 @paaggeli
3 years ago

Hi, first time to contribute.
@jeremyfelt about the esc_html() to run after the new filter, there is another filter wp_title, which runs the esc_html() before the capital_P_dangit(), and I decided to left the same order also for the new filter.

#3 @paaggeli
3 years ago

  • Keywords has-patch added; needs-patch removed

#4 @SergeyBiryukov
3 years ago

  • Milestone changed from Future Release to 5.8

@audrasjb
3 years ago

#5 @audrasjb
3 years ago

In 51643.1.diff:

  • Switch a bit the order of the document_title parameter for better consistency.
  • Fix few coding standard issues in the DocBlock comments
  • Add a version number in the @since declaration

Patch should be good to go.

#6 @desrosj
3 years ago

  • Owner set to desrosj
  • Status changed from new to reviewing

#8 @desrosj
3 years ago

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

In 51019:

Formatting: Introduce the document_title filter.

In wp_get_document_title(), the returned value is currently passed directly through wptexturize(), convert_chars(), and capital_P_dangit(), and is done so after the document_title_parts` filter is run.

This makes it impossible to fully control the output of wp_get_document_title() and is inconsistent with how other similar text is processed with these functions.

This commit introduces the document_title filter, which is run immediately before returning the results of the wp_get_document_title() function and moves the three formatting functions mentioned above to the new filter hook. This allows developers to further modify the title after being prepared by WordPress, or to modify the functions hooked to this filter as they wish.

Props dragunoff, jeremyfelt, paaggeli, audrasjb.
Fixes #51643.

#10 @audrasjb
3 years ago

  • Keywords needs-dev-note added
Note: See TracTickets for help on using tickets.