Make WordPress Core

Opened 8 years ago

Closed 2 months ago

#39740 closed defect (bug) (wontfix)

Twenty Seventeen: Allow child themes to use front-page.php when front page is set to "Your Latest Posts"

Reported by: johnnyb's profile johnnyb Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.7
Component: Bundled Theme Keywords: dev-feedback close
Focuses: template Cc:

Description

What's Happening:

If a child theme of Twenty Seventeen has a front-page.php file, and the Reading settings are set to have "Your Latest Posts" as the front page, Twenty Seventeen's twentyseventeen_front_page_template() function, called on the frontpage_template filter, sets the template to an empty string instead of the child theme's front-page.php, so index.php gets used.

What I expect:

I expect the child theme's front-page.php to be used if present, no matter what the Reading settings are for the front page, as described in the codex. This is the default behaviour.

Relevant Code:

Here's TwentySeventeen's twentyseventeen_front_page_template() function, for reference:

/**
 * Use front-page.php when Front page displays is set to a static page.
 *
 * @since Twenty Seventeen 1.0
 *
 * @param string $template front-page.php.
 *
 * @return string The template to be used: blank if is_home() is true (defaults to index.php), else $template.
 */
function twentyseventeen_front_page_template( $template ) {
	return is_home() ? '' : $template;
}
add_filter( 'frontpage_template',  'twentyseventeen_front_page_template' );

Link to location in code viewer.

Change History (11)

#1 follow-up: @ocean90
8 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed
  • Version changed from 4.7.2 to 4.7

Hello @johnnyb, thanks for your report. This was already mentioned in #39299 with a possible solution so I'm closing this ticket as a duplicate.

#2 in reply to: ↑ 1 @johnnyb
8 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Replying to ocean90:
Hi! Thanks for the quick reply. I'm going to argue my case for a fix a bit more:

I'm not convinced this is a duplicate, because:

1) A default theme shouldn't break WP's template hierarchy.

2) The solution isn't as simple as the "one line" it claims. This actually works, but I believe shouldn't be needed:

function jb_hey_twentyseventeen_dont_filter_frontpage_template_wrongly() {
	remove_filter( 'frontpage_template',  'twentyseventeen_front_page_template' );
}
add_action( 'after_setup_theme', 'jb_hey_twentyseventeen_dont_filter_frontpage_template_wrongly' );

3) This is kind of semantics, but this describes a different behaviour than #39299, (if I'm reading it right, that reporter wants to be able to choose a template in the Edit Page UI), so shouldn't this should be 'wontfix' if it's not going to be fixed?

If this is a case of a lack of manpower to fix an edge case, I'm willing to try to come up with a patch.

#3 @sami.keijonen
8 years ago

If I remember correctly frontpage_template filter is used because otherwise user would have to choose Custom Page Template as a static front page. That's an extra step (and hard/confusing for new users) for user.

  1. Does it break WP's template hierarchy? In a sense yes but setting up a front page in WP is kind of broken for business type of themes. This is the current best solution which still respect user Front page displays settings.
  2. I'd use home.php in my child theme for custom "My latest post" design.
  3. Yes ticket #39299 is different because it's about allowing child themes using custom page template as static front page. In use it on my own themes for that reason.

#4 @davidakennedy
8 years ago

Thanks for the report @johnnyb! And thanks for the context @sami.keijonen – you're exactly right. That was done to be nicer for users in terms of site setup. That said, I can see why child themes would expect the default behavior. :)

@johnnyb What is your use case? Why does your child theme of Twenty Seventeen want to use front-page.php for the posts page if set as front? It will work loosely, but there will be some issues with featured images.

I'm open to fixing this, and making it better so long as we don't break existing sites. We probably just can't turn it off for all child themes because they could want the existing functionality. That leads me to want to leave it as is, and let child themes turn off Twenty Seventeen's changes if need be. But again, open to ideas and suggestions.

Last edited 8 years ago by davidakennedy (previous) (diff)

#5 @johnnyb
8 years ago

Hi @davidakennedy :

My use-case is a bit weird. I'm working on a website with a highly-customized front page - it pulls several posts of a Custom Post Type and puts them together with a pile of animations & effects, so the front page is largely hand-coded, with bits of content coming from the CPT, but for the rest of the site we're using TwentySeventeen to give us templates, (it's great we can set the header image and be nearly done).

I want the fancy front page to be as performant as possible, so I'm using the pre_get_posts action to modify the main WP_Query before it's executed and tell it to get the special front-page content. Since WordPress relies on the results of the main query to help it choose between front-page.php and index.php when the Front Page setting is set to a Page, and I'm changing the query so that front page is never queried, I need to have the Reading setting set to "Your Latest Posts" in order for my front-page.php to work, (there seems to be a WP bug to file somewhere - maybe something like "Stop is_front_page() from relying on the WP Query"

I'm open to fixing this, and making it better so long as we don't break existing sites. We probably just can't turn it off for all child themes because they could want the existing functionality.

It sounds like we're between a rock & a hard place here. FIxing this is likely to cause more outcry than doing nothing. However, I can tell you, it's really frustrating to read the docs & WordPress code, understand that I'm doing everything "right," and find that a WP-blessed default theme is changing that well-documented behaviour, and loose a bunch of time to that.

#6 @davidakennedy
8 years ago

Hey @johnnyb!

Thanks for the detailed explanation. All that makes sense, and I'm really sorry you wasted time because of this. That stinks!

That said, I'm not sure of a possible fix here. We can detect child themes and if the front page is the posts page, but there isn't a great way to know if front-page.php is in use. If there was a reliable way that we knew if child theme makers did this, they want this functionality off, we could explore it. Maybe the best thing to do here is to updates the documentation to make this more clear?

I'm open to other ideas though, before closing this.

#7 @SergeyBiryukov
8 years ago

  • Milestone set to Awaiting Review

#8 @johnbillion
7 years ago

  • Focuses template added
  • Keywords dev-feedback added
  • Priority changed from normal to high

I just stumbled across this and lost quite a bit of time figuring out why the front-page.php template was not loading when, according to the template hierarchy, it should be.

Reading through the comments above, I understand why this was done, but a default theme breaking the default template hierarchy by removing templates from it demonstrates a lack of trust in the template hierarchy. If a default theme can't operate as intended without removing templates from the hierarchy, then how do we expect other themes to?

From https://make.wordpress.org/core/2016/09/09/new-functions-hooks-and-behaviour-for-theme-developers-in-wordpress-4-7/:

It’s important to remember that the consistency of the template hierarchy in WordPress is what makes standardised theme structures possible. It’s highly recommended that you do not remove templates from the candidate hierarchy using these new filters, unless you’re absolutely certain of what you’re doing.

#9 @poena
18 months ago

  • Priority changed from high to normal

I am moving this to priority normal since this theme is no longer bundled with new WordPress releases.

#10 @karmatosed
2 months ago

  • Keywords close added

I would like to have a consideration on how we move forward this.

  • It has been a while and whilst that shouldn't be a reason to close, it might mean any change made could negatively impact.
  • This theme is no longer bundled.

As a result, my recommendation would be not to consider fixing this for now and close. That said, if someone would like to provide a patch or counter that I am more than happy to reconsider.

#11 @karmatosed
2 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

I am going to follow through and close this, thank you everyone.

Note: See TracTickets for help on using tickets.