HI @all,

I must mentioned that the following code block in the pathauto_create_alias() function works incorrectly when you try to add aliases for the same source path, but in different languages during the bulkupdate operation:

<?php
function pathauto_create_alias($module, $op, $placeholders, $src, $entity_id, $type = NULL, $language = '') {

 
// ... more code

  // Special handling when updating an item which is already aliased. 
 
$pid = NULL;
 
$old_alias = NULL;
  if (
$op == 'update' or $op == 'bulkupdate') {
    if (
variable_get('pathauto_update_action', 2) == 0) {
     
// Do nothing
     
return '';
    }
   
$update_data = _pathauto_existing_alias_data($src);
   
$pid = $update_data['pid'];
   
$old_alias = $update_data['old_alias'];
  }

 
// ... more code

}
?>

So the $update_data is loaded without respecting the passed $language variable... So the new alias is always considered an existent alias, if another language already uses the same alias. Is this what it should work like?

Example

The alias value is the same in english and german.
The first call sets the alias in english:

<?php
pathauto_create_alias
('module', 'bulkupdate', array(), 'the/path', 'entity_id', 'type', 'en');
?>

The second call tries to set the alias in german:

<?php
pathauto_create_alias
('module', 'bulkupdate', array(), 'the/path', 'entity_id', 'type', 'de');
?>

Unfortunately the second call wonÄt create a german alias, because the $update_data used contains the english alias and so the system treats the german alias as already existent.

I'd appreciate to see this fixed as soon as possible, as I really need the possibility to update paths with same source but in different languages.

Thanx in advance & cheers

hctom

Files: 
CommentFileSizeAuthor
#25 739416-pathauto-existing-alias-data-language-D7.patch7.36 KBDave Reid
PASSED: [[SimpleTest]]: [MySQL] 136 pass(es).
[ View ]
#24 739416-pathauto-existing-alias-data-language-D7.patch7.4 KBDave Reid
PASSED: [[SimpleTest]]: [MySQL] 136 pass(es).
[ View ]
#23 739416-pathauto-existing-alias-data-language-D7.patch3.68 KBDave Reid
PASSED: [[SimpleTest]]: [MySQL] 125 pass(es).
[ View ]
#21 739416-pathauto-existing-alias-data-language-D7.patch3.68 KBDave Reid
FAILED: [[SimpleTest]]: [MySQL] 121 pass(es), 4 fail(s), and 2 exception(es).
[ View ]
#9 pathauto-739416.patch8.23 KBhctom
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in pathauto-739416_0.patch.
[ View ]
#8 pathauto-739416.patch8.23 KBhctom
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in pathauto-739416.patch.
[ View ]

Comments

klonos’s picture

Please take a look at these three issues:

#357185: template base_path() code - breaking il8n menu links
#358315: drupal_lookup_path() not respects alias' order
#269877: path_set_alias() doesn't account for same alias in different languages

Are they similar to your issue or perhaps one of them is the same (duplicate)? Especially pay attention to this comment of mine:

http://drupal.org/node/357185#comment-2700610

Let me know.

hctom’s picture

@klonos:

As far as I see those issues deal with different things... In my case the alias records are not even tried to be created in the database because the $updata_data holds wrong information that does not repsect the passed language for the new alias. So there are no DB errors or DB records in the database for the second alias of the example.

cheers

hctom

klonos’s picture

ok then. It was just a thought...

thank you for taking the time to reply.

hctom’s picture

no problem... and as far as I examined the problem, extending the _pathauto_existing_alias_data() function signature with an optional language argument and using this while querying the database for existent aliases should solve the problem.

cheers

hctom

hctom’s picture

Here's a draft how the function should look like:

<?php
function _pathauto_existing_alias_data($src, $language = NULL) {
 
$output = array(
   
'pid' => '',
   
'old_alias' => ''
 
);
 
$args = array($src);
 
$sql = "SELECT pid, dst FROM {url_alias} WHERE src='%s'";
  if (!
is_null($language)) {
   
$args[] = $language;
   
$sql .= " AND langauge = '%s'";
  }
 
$result = db_query($sql, $args);
  if (
$data = db_fetch_object($result)) {
   
// The item is already aliased, check what to do...
   
switch (variable_get('pathauto_update_action', 2)) {
     
// Replace old alias - remember the pid to update
     
case 2:
      case
3:
       
$output['pid'] = $data->pid;
     
// Add new alias in addition to old one
     
case 1:
       
$output['old_alias'] = $data->dst;
        break;
     
// Do nothing
     
case 0:
      default:
        break;
    }
  }
  return
$output;
}
?>

And pathauto_create_alias() should then call like this:

<?php
function pathauto_create_alias($module, $op, $placeholders, $src, $entity_id, $type = NULL, $language = '') {

 
// ... more code

  // Special handling when updating an item which is already aliased.
 
$pid = NULL;
 
$old_alias = NULL;
  if (
$op == 'update' or $op == 'bulkupdate') {
    if (
variable_get('pathauto_update_action', 2) == 0) {
     
// Do nothing
     
return '';
    }
   
$update_data = _pathauto_existing_alias_data($src, $language);
   
$pid = $update_data['pid'];
   
$old_alias = $update_data['old_alias'];
  }

 
// ... more code

}
?>
hctom’s picture

Component:Code» I18n stuff

Are there any news from the maintainers on this? I'd appreaciate to hear from you, as this is quite urgent, because this bug really destroys my pathauto behavior... and the site that uses the module has to be launched on April 1st

I'd appreciate any feedback

cheers

hctom

greggles’s picture

Thanks for your bug report and for helping make this a better module. Could you provide this as a patch?

Also, we are trying to get simpletests for everything we add. So, if you could write a simpletest that shows this bug and then runs properly when your patch is applied that would help us understand the problem and be more confident in your solution.

hctom’s picture

StatusFileSize
new8.23 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in pathauto-739416.patch.
[ View ]

Hi @all,

I finally got my first patch done :) This one includes the function signature changes of _pathauto_existing_alias_data() and all changed calls to it. I also added a test (in the unit test case) for the new functionality (including a test module that provides a language dependant token to test against - I guess it is not in the right place, but I am not that familiar with CVS).

To reprodcue the problems occuring without my changes, just apply the patch and revert the pathauto.inc to its base state. When you then unit test the module, you will see, that only the last alias in the line is created for the same path.

I hope this helps and will make it into the module as soon as possible :)

Cheers

hctom

hctom’s picture

StatusFileSize
new8.23 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in pathauto-739416_0.patch.
[ View ]

Damn... I uploaded the wrong file... so here comes the real patch :)

... And I also forgot to mention: This interferes a little with the known path sorting problem of the path module... but there is a comment in _pathauto_multilingual_test_languages() in the pathauto_multilingual_test.module file, describing the issue.

cheers

hctom

greggles’s picture

Status:Active» Needs review

Great work! I haven't reviewed the patch/test but really appreciate that you have taken the time to provide both!

Last step: when you upload a patch that you think is in a good state be sure to mark the status as "needs review."

hctom’s picture

You're welcome! I'll keep the status change in mind next time :)

hctom’s picture

hmmm.... I'm a little bit confuised why nothing is happening here... am I missing something, or shouldn't this be reviewed at last by somebody?

I'm quite disappointed that my work does not get recognized or even processed as this is really an issue that in my opinion should be fixed as soon as possible...

hctom

sirian’s picture

I agree. It's issue is bugging all my internationalized sites. I don't understand why this patch wasn't included in the latest release of pathauto. Is there a Why?

ufku’s picture

the patch works!

risca’s picture

Great! I was really looking for it. I try also others but this is the only one that has worked properly.

zualas’s picture

@risca

I checked your website http://risca.no-ip.info, tried to look at the About page in different languages. The path still doesn't seem to be right for you. Try this:
1) go to your website risca.no-ip.info, click on the About page. The path will be http://risca.no-ip.info/en/content/about
2) Switch the language to Italian. The path will be http://risca.no-ip.info/it/content/about
3) hover your mouse over the About link. The path will be http://risca.no-ip.info/it/node/50 (when you click on it, it gets the correct alias though)

I will open a new issue specifically for the hyperlinks on the page.

risca’s picture

@ zualas
Here more infos:
I am currently using the patch as at post n. 9.
As you can now check the page "about" is now working on both translation (ITA-ENG). The problem, that still affects me, is that the patch works only with new pages, it doesn't fix the previous one. In fact what I did was first to apply the patch, then to translate the page "about" to ENG but, exactly as you've already noticed/debug, it hasn't work. Therefore I try to delete it completely (both the ITA and the ENG version) and to create it again and so it starts working.
Keeping trying I can confirm that the patch works only with new pages and that it doesn't still allow to translate the old one (the one that I have written before to apply the patch).

I think it is a problem in updating the database but I have no clue on how to solve it, I'm not a programmer... Anyway if you need to know about logs and files, just ask me.

---EDIT---
It only works if the new page that I create is already set in a language. Meanwhile if it is "language neutral" and I decide to translate it later on (making it ITA and adding a ENG translation) it wont work and will cause the problem properly described by you:
1) go to your website risca.no-ip.info, click on the About page. The path will be http://risca.no-ip.info/en/content/about
2) Switch the language to Italian. The path will be http://risca.no-ip.info/it/content/about
3) hover your mouse over the About link. The path will be http://risca.no-ip.info/it/node/50 (when you click on it, it gets the correct alias though)

ufku’s picture

Status:Needs review» Reviewed & tested by the community

@risca
I think you should also be using the patch at #269877: path_set_alias() doesn't account for same alias in different languages

These two issues needs to be fixed in order to get same aliases work for multiple languages.

risca’s picture

Sorry, but I notice just now that it is already working. The funny thing was that I was surfing on the website using my profile which had the preferred language set to Italian, therefore I was redirect to any Italian translation, if any available. I try instead to set personal user's language to English and in fact I've been redirect straight to every ENG page.
Sorry sorry sorry.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, pathauto-739416.patch, failed testing.

Dave Reid’s picture

Version:6.x-1.x-dev» 7.x-1.x-dev
Status:Needs work» Needs review
StatusFileSize
new3.68 KB
FAILED: [[SimpleTest]]: [MySQL] 121 pass(es), 4 fail(s), and 2 exception(es).
[ View ]

Re-rolled and improved. Doesn't include the test from the latest patch because there's got to be an easier way to test this.

Status:Needs review» Needs work

The last submitted patch, 739416-pathauto-existing-alias-data-language-D7.patch, failed testing.

Dave Reid’s picture

Status:Needs work» Needs review
StatusFileSize
new3.68 KB
PASSED: [[SimpleTest]]: [MySQL] 125 pass(es).
[ View ]

Odd. Apparently I can't use :language-none as a placeholder. Oh well. Modified the patch.

Dave Reid’s picture

StatusFileSize
new7.4 KB
PASSED: [[SimpleTest]]: [MySQL] 136 pass(es).
[ View ]

Worked on an easy test case that tests a node with two different language aliases gets the proper language alias deleted when the node is updated and alias changed.

Dave Reid’s picture

StatusFileSize
new7.36 KB
PASSED: [[SimpleTest]]: [MySQL] 136 pass(es).
[ View ]

Without a debugging line left in....

Dave Reid’s picture

Confirmed that rolling back the patch but leaving the test file confirms a failure, so this should be fixing it properly. This would be great if someone else in this issue could test this patch on D7.

Dave Reid’s picture

Status:Needs review» Fixed

Finally got this committed to both D7 and D6-2 branches with working tests.
http://drupal.org/cvs?commit=397220
http://drupal.org/cvs?commit=397222

Thank you everyone for your help on this issue.

Status:Fixed» Closed (fixed)

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

clashar’s picture

I can't see Pathauto version of July 25, 2010 or later
Please confirm that patch is committed, the last 7-x version is dev-version of 2011-Jun-22

clashar’s picture

Status:Closed (fixed)» Active

and I can't apply
739416-pathauto-existing-alias-data-language-D7_0.patch
739416-pathauto-existing-alias-data-language-D7_1.patch
739416-pathauto-existing-alias-data-language-D7_2.patch

to latest Pathauto dev 2011-Jun-22
as patches refer to 5 July version

Dave Reid’s picture

Status:Active» Closed (fixed)

This is already included in the latest releases.

clashar’s picture

please explain in which 2011-Jun-22?

Dave Reid’s picture

You are greatly over thinking things. All you have to do is download one of the latest releases listed on the project homepage (http://drupal.org/project/pathauto). For example, the 7.x-1.0-rc2 release has a date of June 16, 2011, which is later than the date the fix was committed in June 2010.

BrianLewisDesign’s picture

Title:_pathauto_existing_alias_data() should respect passed language» template base_path() code - breaking il8n menu links

il8n prefix was removed from menu links in my template.php file:

$href = $link['href'] == "<front>" ? base_path() : base_path() . drupal_get_path_alias($link['href']);

Fixed it by sticking $languge->prefix in the path:

global $language;
$prefixx = '';
if ( $language->prefix ) { $prefixx = $language->prefix."/"; }
$href = $link['href'] == "<front>" ? base_path() : base_path() . $prefixx . drupal_get_path_alias($link['href']);

I guess the best way would be to use l() or url()? This worked easily for me in my template.

vflirt’s picture

Status:Closed (fixed)» Needs work

Hi,

it is all great but the function _pathauto_existing_alias_data orders the language DESC which should be wrong.
as you have language 'und' and 'en' for example then if I try yo take the current alias for language 'en' it will return me the one for 'und' and that is wrong.

The correct order is : $order = $language < LANGUAGE_NONE ? 'ASC' : 'DESC'; as it is in path.inc of the core

If i am wrong please show me how to get the current alias for language 'en' when there is alias for language 'und'.

Kind Regards,
Dobromir

ricovandevin’s picture

I agree with vflirt that the order can be wrong. In some cases however it can be correct. There should be distinction between $language < LANGUAGE_NONE and $language > LANGUAGE_NONE. I have created a patch for that in #2194431: _pathauto_existing_alias_data() should return language-specific alias if there is one.

  • Dave Reid committed c0a2c03 on 8.x-1.x
    #739416 by Dave Reid, hctom | risca, klonos: Fixed...