theme_get_suggestions() fails to return 'page__front' suggestion on front page [#674784]

In Drupal 6, to support a front page theme template, we had:

  if (drupal_is_front_page()) {
    $suggestions[] = 'page-front';
  }

inside template_preprocess_page(), allowing us to use page-front.tpl.php for a filename. In Drupal 7, this has been moved to template_page_suggestions(), which uses the following code instead:

  if (drupal_is_front_page()) {
    $suggestions[] = $suggestion . '-front';
  }

$suggestion, here, is pre-built from the values of arg(). arg() always has a value. The end result is that, in a default install of Drupal, where site_frontpage defaults to 'node', arg() returns 'node'. This 'node' gets stored in $suggestion. Thus, in a default install of Drupal, there is no 'page-front.tpl.php', only 'page-node-front.tpl.php'.

This is problematic: since the $suggestion is always based on arg(), it means that that filename for the "front page of the theme" will always be different, depending on the value of site_frontpage. Once a custom theme has been made with a front page template, one would have to be very careful about changing site_frontpage to any other value (besides a node), since it would cause the theme filename to change (i.e., change site_frontpage to /whee, which is a menu callback in whee.module, and the front page filename becomes 'page-whee-front.tpl.php'.)

Setting as critical since it a) changes (good) behavior from earlier versions of Drupal, b) makes it impossible to have a "standard" filename for the front page template, c) makes it nearly impossible for shipped themes to ship a front page template without writing their own suggestion code.

Comment	File	Size	Author
#12	674784_1.patch	2.15 KB	naxoc
#12
#11	674784.patch	2.03 KB	cha0s
#11
#8	674784.patch	2.12 KB	naxoc
#8
#7	theme-page-front.patch	1.19 KB	mr.baileys
#7
#2	page-front-674784-2.patch	1.02 KB	JohnAlbin
#2
#1	page-front-674784-1.patch	909 bytes	JohnAlbin
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

JohnAlbin

he/they/他

English

Taipei, Taiwan

CreditAttribution: JohnAlbin commented 5 January 2010 at 14:36

Status:

Active

» Needs review

File	Size
page-front-674784-1.patch	909 bytes

This should do the trick.

Comment #2

JohnAlbin

he/they/他

English

Taipei, Taiwan

CreditAttribution: JohnAlbin commented 5 January 2010 at 14:46

File	Size
page-front-674784-2.patch	1.02 KB

In IRC, Morbus gave me a comment line to add to the patch to help explain $suggestion vs. $prefix.

Comment #3

Morbus Iff

they/them

CreditAttribution: Morbus Iff commented 5 January 2010 at 14:49

Status:

Needs review

» Reviewed & tested by the community

Comment #4

jensimmons CreditAttribution: jensimmons commented 5 January 2010 at 15:26

+1 Fix this bug! :) We *need* page-front.tpl.php......

Comment #5

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 5 January 2010 at 18:03

Status:

Reviewed & tested by the community

» Needs work

Oops. :\

IMO we should write some tests for this so we never break this again.

Comment #6

mr.baileys

Dutch

🇧🇪 (Ghent)

CreditAttribution: mr.baileys commented 6 February 2010 at 11:38

Needs a re-roll because of #678714: Unify use of theme hook / template suggestions, fix clobbering problems, and improve suggestion discovery performance

Comment #7

mr.baileys

Dutch

🇧🇪 (Ghent)

CreditAttribution: mr.baileys commented 6 February 2010 at 12:23

Status:

Needs work

» Needs review

File	Size
theme-page-front.patch	1.19 KB

Rerolled to keep up with HEAD.

Comment #8

naxoc CreditAttribution: naxoc commented 8 February 2010 at 23:34

File	Size
674784.patch	2.12 KB

Added a test.

Comment #9

Frando CreditAttribution: Frando commented 9 February 2010 at 00:21

+++ modules/simpletest/tests/theme.test
@@ -39,6 +39,21 @@ class TemplateUnitTest extends DrupalWebTestCase {
+    // Set $_GET['q'] to node because theme_get_suggestions() will query it to
+    // see if we are on the front page.
+    $_GET['q'] = 'node';

I think this should use variable_get('site_frontpage', 'node').

+++ modules/simpletest/tests/theme.test
@@ -39,6 +39,21 @@ class TemplateUnitTest extends DrupalWebTestCase {
+    $this->assertEqual($suggestions, array('page__node', 'page__front'), t('Front page template was suggested.'));

I think we should just check whether page__front is in the $suggestions array, otherwise the test needs to be updated whenever a new suggestion is added, which is not what we want, I guess.

Powered by Dreditor.

Comment #10

Frando CreditAttribution: Frando commented 9 February 2010 at 00:29

Status:

Needs review

» Needs work

Comment #11

cha0s CreditAttribution: cha0s commented 9 February 2010 at 13:51

Status:

Needs work

» Needs review

File	Size
674784.patch	2.03 KB

Updated.

Comment #12

naxoc CreditAttribution: naxoc commented 9 February 2010 at 21:14

File	Size
674784_1.patch	2.15 KB

Found an error in the phpdoc comment and updated the use of variable_get to be used as the argument for theme_get_suggestions too.

Comment #13

cha0s CreditAttribution: cha0s commented 10 February 2010 at 03:52

Good call. Test bot, we choose YOU!!

Comment #14

yoroy CreditAttribution: yoroy commented 19 February 2010 at 01:22

Duplicated page.tpl.php in core Garland, renamed to page-front.tpl.php and added some html that made it different from page.tpl.php. Cleared cached but not seeing anything different on /node.

To be sure, I set the front page to be node/1, but no changes visible there either.

*edit* To be really sure I duplicated/renamed Stark and put that into sites/all/themes. Added page.tpl.php and added some blabla. Empty cache: blabla shown. Added a page-front.tpl.php with added yadda-yadda. Still blabla was shown on front page.

Comment #15

cha0s CreditAttribution: cha0s commented 19 February 2010 at 01:14

Status:

Needs review

» Needs work

In addition to fixing this we need to figure out why the tests pass if it's still broken, and fix the tests...

Comment #16

naxoc CreditAttribution: naxoc commented 20 February 2010 at 12:03

I can't get node-article.tpl.php tpl be found either. The suggestions are added OK - and that is why no tests fail, but they are not actually used. I suspect the drupal_find_theme_templates function is getting something wrong, but I am not at all sure.

But I am pretty sure that no suggested templates will be used as it is right now. Looking into it.

Comment #17

cha0s CreditAttribution: cha0s commented 22 February 2010 at 13:15

I'm assuming we can install (hidden) themes using simpletest? If so, we should consider adding themes to *actually* fail currently, until we can track down and fix the root cause. If no one jumps on that I'll get it rolling.

The theme's templates will have some kind of key in them 'Testing page-front.tpl.php' and then visiting those pages through SimpleTest programatically should cause that text to be found.

Comment #18

catch

he/him

English

CreditAttribution: catch commented 23 February 2010 at 07:44

Status:

Needs work

» Postponed (maintainer needs more info)

Suggestions are now page--front.tpl.php and node--article.tpl.php, can't find the issue of hand. Please confirm those work, if this hasn't been added to the theme upgrade guide yet, we should bump that issue since i'm sure more people will get caught out by this.