Download & Extend

locale_block: incorrect path for frontpage

Project:Drupal core
Version:6.x-dev
Component:locale.module
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

When I'm on frontpage, I expect the icons url to point to http://mysite.com and the http://mysite.com/id.

Instead i got http://mysite.com/nwhome/home and http://mysite.com/id/nwhome/home respectively.
I'm setting the frontpage to nwhome/home. It's a content type specially crafted for frontpage.

Anybody experiencing this issue ?

PS: Take a look at http://alpha.yasati.org for more info.

Comments

#1

Project:Language icons» Drupal core
Version:6.x-1.0-beta1» 6.4
Component:Code» locale.module
Category:bug report» support request

Language Icons only add icons to the links, it doesn't generate the links themselves.

#2

Category:support request» bug report
Status:active» needs review

Thanks, Fresco for pointing out that it's the locale module problem..
Though I still think this is a bug..

Attached is a one line patch for locale module to fix this problem..

Please review :)

AttachmentSizeStatusTest resultOperations
locale.blocks.links_.patch574 bytesIgnored: Check issue status.NoneNone

#3

Version:6.4» 7.x-dev

Right, of course. Anyway, if you want to have it fixed, it should go to 7.x (and then backported to 6.x) - of course, only if the bug still exists in the 7.x code base.

First off: A quick work around for your sites could be to use the Global Redirect module until the issue is fixed.

Second: Perhaps something smarter than $_GET['q'] == variable_get('site_frontpage', 'node') ? '' : $_GET['q'] could be used to determine if we're already in the frontpage? Obviously, when you're on "site/" or "site/en", $_GET['q'] will be '' - that is, not 'node' or any other string set as the site_frontpage variable. Or perhaps we could just remove the logic all together, as is done in the patch? This is what is done anyway, if not on the front page, so...

#4

well, let's just keep it simple..
As actually I don't quite follow you :P

Btw, is there any obvious bugs by doing it this way?

#5

how about drupal_is_front_page() instead? Btw, it looks like this has already been changed in HEAD, so there must be another issue dealing with this, but I couldn't locate one.

Currently the code reads

<?php
   
foreach ($languages[1] as $language) {
     
$links[$language->language] = array(
       
'href'       => $_GET['q'],
        ...
?>

#6

The code seems to be from #141996: Language switcher block, which used 'href' => $_GET['q']... and it doesn't seem to have changed in neither 6.x or 7.x.

(And now in the proper issue to boot.)

#7

have the same problem, thank you I'll try it first ...

#8

Title:Path for FrontPage» locale_block: incorrect path for frontpage

The proper way is indeed to call drupal_is_front_page(), and to use '' when it is. url() will take care of the rest.

AttachmentSizeStatusTest resultOperations
295626-locale-block-front.patch728 bytesIgnored: Check issue status.NoneNone
295626-locale-block-front-d6.patch739 bytesIgnored: Check issue status.NoneNone

#9

Thanks Damien,

Patch tested on 6.4.
Seems good.

#10

Status:needs review» reviewed & tested by the community

The patch looks good to me and the OP is satisfied. Marking RTBC.

#11

Status:reviewed & tested by the community» needs review

Modified patch to let i18n play nicely with the locale block.

Any review??

AttachmentSizeStatusTest resultOperations
locale.block_.front_.patch1.18 KBIgnored: Check issue status.NoneNone

#12

Status:needs review» needs work

Patch failed to apply. More information can be found at http://testing.drupal.org/node/14267. If you need help with creating patches please look at http://drupal.org/patch/create

#13

Cool.. QA Patch tester :P

Let's try this..

AttachmentSizeStatusTest resultOperations
locale.block_.patch1.15 KBIgnored: Check issue status.NoneNone

#14

Status:needs work» needs review

#15

Status:needs review» needs work

Patch failed to apply. More information can be found at http://testing.drupal.org/node/14269. If you need help with creating patches please look at http://drupal.org/patch/create

#16

Status:needs work» needs review

Another try..

It it still doesn't work.. I'll let others give it a shot.

AttachmentSizeStatusTest resultOperations
locale.block_.patch1.15 KBIgnored: Check issue status.NoneNone

#17

Status:needs review» needs work

Patch failed to apply. More information can be found at http://testing.drupal.org/node/14270. If you need help with creating patches please look at http://drupal.org/patch/create

#18

subscribe. i didn't test this yet, but upon looking into code, i don't like that we are adding yet another SQL query for determining the path. Is there another solution?

#19

Status:needs work» needs review

please try this patch out.

AttachmentSizeStatusTest resultOperations
locale_295626.patch1.82 KBIgnored: Check issue status.NoneNone

#20

Status:needs review» needs work

@drewish:

<?php
+    $path  = drupal_is_front_page() ? '<front>' : $_GET['q'];
?>

Please remove the spaces after the $path.

@meba:

We are not adding an SQL query. drupal_is_front_page() is cached, and it is called anyway (for example in menu_get_active_breadcrumb().

#21

Damien Tournoud, at this point for a one character change you could re-roll the patch as easily as i could... or heck you could just edit the patch.

#22

Status:needs work» reviewed & tested by the community

@drewish: why are you not allowing me to be lazy?

Here is a proper patch, I confirmed again it works as intended. Because the patch is trivial and several people already looked at it, I'm marking this as ready to go in.

AttachmentSizeStatusTest resultOperations
295626-locale-block-frontpage.patch1.84 KBIgnored: Check issue status.NoneNone

#23

Version:7.x-dev» 6.x-dev

Thanks guys, comitted. Marking down to 6.x for consideration.

#24

Status:reviewed & tested by the community» fixed

Thanks, committed to Drupal 6.x.

#25

Status:fixed» closed (fixed)

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