From drupal_is_front_page():

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

Couldn't we store the normal path in the variable in the first place? I cannot see any use case where you would like the variable to contain a path alias.

CommentFileSizeAuthor
#25 path-front-page-D6.patch2.71 KBrdrh555
#18 fix_head.patch722 bytescweagans
#13 drupal.path-front-page.13.patch5.69 KBsun
#9 drupal.path-front-page.9.patch5.69 KBsun
#8 drupal.path-front-page.8.patch2.99 KBsun
#1 613858-site-frontpage-normalize-D7.patch3.62 KBDave Reid
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Active » Needs review
FileSize
613858-site-frontpage-normalize-D7.patch3.62 KB

We normalize menu paths on saving but display aliases, so why not do the same here.

Status: Needs review » Needs work

The last submitted patch failed testing.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented 
+++ modules/system/system.admin.inc	25 Oct 2009 03:50:58 -0000
@@ -1313,7 +1313,7 @@ function system_site_information_setting
-    '#default_value' => 'node',
+    '#default_value' => drupal_get_path_alias('node'),

site_frontpage is really an internal path. There is no need to try to convert it to an external path here (plus this will only work for the default value, not for the variable value).

I'm on crack. Are you, too?

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

Yes, I know, but this is only for display purposes. For instance if the user has set an alias for the node path as 'frontpage' they'll see 'frontpage' here. I'm pretty sure this is how we handle menu link paths as well in the interface. If not, we can toss it out.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

@Dave: your code only apply to the default value of the variable. The value of the variable itself is automatically loaded by system_settings_form().

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

Damn. Wish we could exclude an individual elements from the automatic default loading.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Issue tags: +Performance

subscribing

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Assigned: Unassigned » sun
Status: Needs work » Needs review
FileSize
drupal.path-front-page.8.patch2.99 KB

What's left?

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
FileSize
drupal.path-front-page.9.patch5.69 KB

There are times when we simply can't rely on magic.

If this helper functionality doesn't support it, then we need to go back to old approaches.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

It's really too bad we can't exclude individual elements from the automatic default code.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Assigned: sun » Unassigned
Status: Needs review » Needs work

This removes one query from nearly all pages in HEAD. Although we cache system paths and try to load all at once, we don't do this well for drupal_get_normal_path(), and we don't cache paths without aliases (which /node or a view or panel used for the front page almost always lacks).

HEAD:
Executed 70 queries in 42.97 milliseconds.

Patch:
Executed 69 queries in 41.7 milliseconds.
+ // Get the normal path of the font page.

However

+  // Get the normal path of the font page.

font page.

Otherwise RTBC.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Assigned: Unassigned » sun

Didn't mean to unassign.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Reviewed & tested by the community
FileSize
drupal.path-front-page.13.patch5.69 KB

RTBC as per catch.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Lovely.

Dries’s picture

Dries CreditAttribution: Dries commented

Committed to CVS HEAD. Thanks!

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed
yched’s picture

yched CreditAttribution: yched commented
Title: drupal_is_front_page() should not call drupal_get_normal_path() » [HEAD BROKEN] drupal_is_front_page() should not call drupal_get_normal_path()
Priority: Normal » Critical
Status: Fixed » Active

system_update_N() needed a bump after http://drupal.org/cvs?commit=296276

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
CreditAttribution: cweagans commented
Status: Active » Needs review
FileSize
fix_head.patch722 bytes

Please fix head. It's sorta important =P

ksenzee’s picture

ksenzee
she/her
Location Seattle area
CreditAttribution: ksenzee commented
Status: Needs review » Reviewed & tested by the community

Yes please.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Oopsie. ;)

Committed.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: [HEAD BROKEN] drupal_is_front_page() should not call drupal_get_normal_path() » drupal_is_front_page() should not call drupal_get_normal_path()

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Version: 7.x-dev » 6.x-dev
Status: Closed (fixed) » Patch (to be ported)

This should be backported to D6.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Assigned: sun » Unassigned
rdrh555’s picture

rdrh555 CreditAttribution: rdrh555 commented
Assigned: Unassigned » rdrh555
Status: Patch (to be ported) » Needs review
FileSize
path-front-page-D6.patch2.71 KB

Some things seemed to have changed in the past 8 months and others seem to have worked their way into code naturally(specifically default_value => variable_get(*) ). I've attempted a patch.

dpearcefl’s picture

dpearcefl CreditAttribution: dpearcefl commented
Priority: Critical » Normal
Status: Needs review » Postponed (maintainer needs more info)

Has this issue been fixed in the latest D6?

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Priority: Normal » Major
Status: Postponed (maintainer needs more info) » Needs review

@dpearceMN: Thanks for trying to triage the queue, but please don't mark such issues as needing more info. Obviously, it wasn't committed to D6 yet. This patch needs review and more work.

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.