If you set the front page path to be 'node/1' and you go to 'node/1' or the alias for that node, $is_front returns FALSE when it should return TRUE.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

RobRoy’s picture

Project: PHPTemplate » Drupal core
Version: master » 4.7.2
Component: Code » base system
Status: Active » Needs review
FileSize
656 bytes

Seems to be an issue with drupal_is_front_page(). Moving and attaching a patch for 4.7.2. I tried to avoid any notices so included the extra isset(), not sure if that will do it.

RobRoy’s picture

FileSize
629 bytes

Maybe we don't need that extra isset since it's already true from the first !isset(). Another patch for 4.7.2.

beginner’s picture

Version: 4.7.2 » x.y.z

If the bug exists in cvs, you should provide a patch for cvs first.
I am sure our excellent 4.7 branch maintainer will backport the fix.

RobRoy’s picture

FileSize
602 bytes

Patch for HEAD. I'm pretty sure this won't create a PHP Notice.

beginner’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
660 bytes

tested: the patch works as intended.
I couldn't apply the patch though: the diff must be malformed. I had to apply it manually.
Here is basically the same patch for head, that should apply cleanly.

beginner’s picture

FileSize
659 bytes

same without extra space :)

drumm’s picture

Status: Reviewed & tested by the community » Needs review

Would simply
$_GET['q'] == variable_get('site_frontpage', 'node')
work?

Dries’s picture

I think it would, and it might actually work better in presence of URL aliasing tricks (might not be applicable for front page). Anyway, worth giving a try.

(Do we need a function for this at all? It seems like a trivial test.)

drumm’s picture

I'd like to see a function used by themes instead of the PHPTemplateism of an extra variable that doesn't exist in the original themeable function.

Dries’s picture

drumm: do you mean we should get rid of:

themes/engines/phptemplate/phptemplate.engine:  $variables['is_front'] = drupal_is_front_page();
drumm’s picture

Yep.

Dries’s picture

$_GET['q'] == variable_get('site_frontpage', 'node')

would work depending on how you look at things.

If 'site_frontpage' variable equals 'node' then http://example.com/node will also be considered as the front page. I don't think that would be a problem, so I suggest we go with Neil's suggestion.

RobRoy’s picture

FileSize
1.42 KB

Per the discussion, check this patch for HEAD that removes drupal_is_front_page() and includes the condition in the phptemplate setting of $is_front. Thoughts?

RobRoy’s picture

Scratch that last patch. I misunderstood. We want to keep drupal_is_front_page() but remove the $is_front from phptemplate alltogether?

beginner’s picture

Status: Needs review » Needs work

Unless I'm mistaken, this won't work if q is not set: $_GET['q'] == '', so the extra condition is necessary.

RobRoy’s picture

That's what I thought, but $_GET['q'] == variable_get('site_frontpage', 'node') works just fine when accessing http://www.example.com/ or /node. Just do a var_dump($_GET['q']) in your page.tpl.php to see it.

RobRoy’s picture

What's the word on this? What can I do to make help this get committed? Changes? New patch?

lennart’s picture

in some cases it is more complicated - imagine you have the top-level of a book set as frontpage. Now this patch would solve the problem of the breadcrumb being there when the top-level page in the book is accessed directly.

But if you then choose any child-page 'content' will be injected in the breadcrumb because child-pages have no way of nowing that the parent-page is actually also the frontpage.

This makes it impossible to use book to create a consistent hierarchical with a book as frontpage.

lennart’s picture

to clarify: home in the breadcrum would lead to frontpage, but so would the name of the top-level book page chosen to be front page - when browsing a childpage.

RobRoy’s picture

FileSize
640 bytes

I know this is a small issue, but I'd like to resolve it. Dries, drumm you both want the themes to start using drupal_is_front_page() instead of $is_front? I feel like $is_front is pretty useful in most themes so and is worth keeping in there.

Here is a re-rolled patch against HEAD that just fixes the drupal_is_front_page() function.

RobRoy’s picture

Status: Needs work » Needs review
drumm’s picture

Status: Needs review » Fixed

Committed to HEAD.

Anonymous’s picture

Status: Fixed » Closed (fixed)
Mad Maks’s picture

Version: x.y.z » 4.7.3
Status: Closed (fixed) » Active

This fix causes bug http://drupal.org/node/77294 : the custom front.tpl.php is not used anymore for the frontpage. my front page is node 284 with "front" as alias. on the setting page the frontpage is set to the alias front.

MM

drumm’s picture

Status: Active » Closed (fixed)

There only needs to be one bug open for this. The second one is sufficient.

pwolanin’s picture

Status: Closed (fixed) » Active

For this patch to work, it seems that the front page setting has to be restricted to a system path. I don't think this is clear from the form text, nor do I think that this is validated (can it be?).

With this patch, if I put a path alias as the front page, the link to "home" brings me to the right page, but drupal_is_front_page() returns FALSE.

Mad Maks’s picture

indeed, if i put node/286 as frontpage on the setting page, there is no problem. mysite.com/front and mysite.com/node/286 are using the front.tpl.php. but if i put the alias on the setting page mysite.com/front and mysite.com/node/286 aren't using the front.tpl.php.

@drumm: sorry for the two bugs but i think the other one has to be closed and this one active. i filed first a bug but nobody did respond. today i had the time to track what was causing the problem and i found this bug. i didn't close the other one yet because i thought it was good to write there that i fount cause of the problem.

pwolanin’s picture

Status: Active » Needs review
FileSize
847 bytes

Should this be a new issue? Small patch attached just tried to clarify the settings page.

beginner’s picture

check those issues:

doesn't work with aliases urls
http://drupal.org/node/78398

drupal_is_front_page fails if front page is aliased
http://drupal.org/node/77479

RayZ’s picture

Status: Needs review » Closed (fixed)

@pwolanin: Regarding #26, let's deal with it as a separate issue (http://drupal.org/node/77479) with it's own patches so we can close this one.