Testing this module at multilingual site (path prefix)

I found bug in friendlist_ui.module - All links appended by destination=$_GET['q] wich leads to 404 page

This problem caused by friendlist_ui_destination() function

Suppose beter to use drupal_get_destination() wich works fine! So friendlist_ui_destination() shoul be removed - I found no occurrences of it's usage.

  // Et voila - return the link, ready to be used by themes etc.
  return $popups_body  .   l(t($link_text, array('!rt_name' => $rt->name, '!rt_name_p' => $rt->name_p)),
    $link_href,
    array(
-      'query' => 'destination='. friendlist_ui_destination(),
+      'query' => drupal_get_destination(),
      'attributes' => array(
        'class' => "$popups_style friendlist_ui_${status}_$op",
      )
    )
  );
CommentFileSizeAuthor
dest.patch1.55 KBandypost

Comments

mariusooms’s picture

mariusooms commented

Merc...did you review this, so we can commit it?

Marius

andypost’s picture

andypost
he/him
Primary language Russian
commented

This patch still valid, this is simple

mariusooms’s picture

mariusooms commented
Status: Needs review » Fixed

Committed and thanks!

Marius

mercmobily’s picture

mercmobily commented
Status: Fixed » Active

Hi,

There is a reason why there was friendlist_ui_destination() . Here it is: basically, if a site has URL renaminc, and it's set so that /user/12345 is changed as user/merc , then I would like the links returned by the module to be *url_aliased* rather than not. Otherwise, people who want to absolutely hide user/12345 will be screwed :D

At the moment, drupal_get_destination doesn't do that -- whereas friendlist_ui_destination does, although it might create problems with multilingual sites. I think the trick is to fix friendlist_ui_destination() , _and_ open a feature request with Drupal where you ask them to have a get_destination function which returns an url_aliased URL...

Irakli, what do you think?

Sorry, reopening this one...

Merc.

irakli’s picture

irakli commented

I think Merc has a good point.

This probably needs a little bit more tweaking.

Thanks everybody

mercmobily’s picture

mercmobily commented

Hi,

I reverted the devel version of the module to what it was before the patch.
The best path at this point is to:

1) Fix friendlist_ui_destination() so that it works, for the short term

2) Open an issue in Drupal where we request a better drupal_get_destination(), for the long term

I have never created a multilingual site... andypost, I don't suppose you would be able to give me the simplest possible list of steps to reproduce this by installing a multilingual site (see: "bare minimum") with FriendList?

Thanks a lot,

Merc.

andypost’s picture

andypost
he/him
Primary language Russian
commented

@Merc setup is simple - just add another language (admin/settings/language), choose path prefix type (admin/settings/language/configure), at admin/settings/language/edit/en add path prefix for language (for example en)

Suppose that destination should not be changed because it just remember previous page or current page so no need to analyse. Take a look at http://api.drupal.org/api/function/drupal_get_destination/6

if you already on user profile page (possible with rewrite) drupal_get_destination give you this page!

mercmobily’s picture

mercmobily commented

Hi,

I don't have admin/settings/language -- what module do I have to enable?

Merc.

andypost’s picture

andypost
he/him
Primary language Russian
commented

locale :)

mercmobily’s picture

mercmobily commented

Hi,

OK, sorry... I am a little multi-language-inept!

Merc.

mercmobily’s picture

mercmobily commented

Hi,

*** I am an idiot, ***

You (ALL of you) were correct, got rid of my idiotic function, reverted to drupal_get_destination.

Sorry.

Merc

mercmobily’s picture

mercmobily commented
Status: Active » Fixed
mercmobily’s picture

mercmobily commented
Status: Fixed » Closed (fixed)