Steps to reproduce:

  1. Create a multilingual Drupal site and set language negotiation to Path prefix with language fallback. The idea is to have URLs such as http://yourdomain/es for Spanish for example and http://yourdomain/en for English
  2. Install and activate Facebook status
  3. Go to your user page and create a state
  4. Try to edit it. The redirection does not work.

Explanation:
The links of the edit and delete point to:
http://yourdomain/en/statuses/x/edit?destination=en/user/x
while they should point to:
http://yourdomain/en/statuses/x/edit?destination=user/x

Fix:
I've fixed it on my site by commenting out the following lines in facebook_status.js:

  //Fix bad redirect destinations.
  context.find('.facebook_status_edit_delete a').each(function() {
    //window.location.href doesn't work for sites not in the webroot with weird server configurations.
    var redirectTo = window.location.pathname.substring(Drupal.settings.basePath.length) + window.location.search;
    $(this).attr('href', $(this).attr('href').split('?')[0] +'?destination='+ escape(redirectTo));
  });

However, I believe that these lines are supposed to fix some other bug, so I don't think commenting out these lines can be considered as a permanent fix.

CommentFileSizeAuthor
#2 bad_redirects.patch1.55 KBguillaumev

Comments

icecreamyou’s picture

Status: Active » Postponed (maintainer needs more info)

Why is the correct link user/UID instead of en/user/UID? Based on the pattern you've explained it seems to me like the module is using what should be the correct path.

guillaumev’s picture

StatusFileSize
new1.55 KB

The correct link is user/UID instead of en/user/UID because when you have a link such as en/user/UID, you get redirected to:
http://yourdomain/en/en/user/UID
while you should get redirect to
http://yourdomain/en/user/UID

Attaching patch...

icecreamyou’s picture

As you guessed, that code is there for a reason and will break other things if it's commented out. (Namely, most links with a destination query parameter will redirect to a WSOD if clicked after updating a status via AJAX.) I'm inclined to say that the behavior you're experiencing is actually an error in your .htaccess directives (see this issue). For example if you manually enter yourdomain.com/en/user/UID in your browser it should go to the right place, but if you're having problems with this then it probably doesn't. If that's not the problem I'm going to have to think of a creative solution to this because nothing is coming to me off the top of my head...

guillaumev’s picture

Hi,

When I go to http://yourdomain.com/en/user/UID it works just fine and goes to the right place.

Also, if I click on the Edit link of the fbssc (facebook-style statuses comments) module, it goes to the right place, because the value of destination is user/UID and NOT en/user/UID.

I'm pretty sure this is a bug in the javascript code of the FBSS module and not an issue specific to my site. Have you tried to reproduce the bug ?

icecreamyou’s picture

Have you tried to reproduce the bug ?

What language/URL-rewriting modules are you using?

guillaumev’s picture

I'm using the Internationalization module (http://drupal.org/project/i18n) v 6.x-1.5

icecreamyou’s picture

Sorry for the delay on this, I've been really busy and this requires more work than usual to reproduce, debug, and fix. It's still on my to-do list but I might not get to it for awhile.

donquixote’s picture

I can confirm if destination is "user/x", drupal/locale/i18n will automatically prepend the language code.

For javascript url building it can help to give the script a variable created on server side, such as drupal_get_normal_path($_GET['q']).

Drake’s picture

Same issue.
The confirmation page redirects to "http://yourdomain/en/en/user/UID" instead of "http://yourdomai/en/user/UID"

Hopefully this patch will be available in the next release.

Drake’s picture

Why this patch was not included in the new 2.0 .dev version?

icecreamyou’s picture

Why this patch was not included in the new 2.0 .dev version?

Because there is no "new 2.0 dev version," and there is no patch. (The solution in the original post breaks edit/delete link redirection on all sites, including multilingual ones, after a status has been submitted on a page via AHAH at least once.)

firstov’s picture

I've tried this temporary workaround: Enable module "Rules" from http://drupal.org/project/rules and setup URL redirection action (Page redirect) to "user" on "User edits a status" event
this fixes "page not found" error, so no more /en/en/user/uid (it has some drawbacks if you need to edit other people comments!)

In the meantime I'm looking for this critical bug to be fixed. Unfortunately I'm not a developer, however please let me know if I can provide more details to help.

I'm using the Internationalization module from http://drupal.org/project/i18n on my instance of Drupal 6.

tic2000’s picture

Status: Postponed (maintainer needs more info) » Active

I think the info is enough.
My question is what exactly that piece of javascript code tries to fix/achieve? Because as we can see it doesn't do the best of jobs and I never had to use something like that to get a correct path (which is a wrong path in a multilingual system using prefix).
I'm asking so maybe I can provide a better way to do it.

tic2000’s picture

As I look more at the code and at the issue you pointed to I say that javascript needs to go.

No link in drupal is altered through a javascript to be "fixed". The core comments module links are exactly like this. The fbssc edit/delete comments links are like this. The links on admin/content/node are like this. And all of them work without a need for a javascript script to change them.

You actually provided a fix for a badly configured server, which didn't brake anything because the site didn't use path prefixes, which in return brakes any multilingual site using path prefixes.

icecreamyou’s picture

No. FBSS links are built like any other links in core, but action links (like edit/delete) have the ?destination= parameter. The destination is built dynamically in PHP based on what the current page is, so that for example when you delete a status you are redirected back to the page on which you started. This is standard, and core (and everything else) does it like this.

The problem is that when a status is submitted via AHAH, the view is refreshed by fetching it from the AHAH callback page. This callback page has a different URL than the page from which the user submitted the status update, so the destination parameter points to the wrong place. Core doesn't have JavaScript to fix this because it assumes that its links won't be updated via AHAH. However, if you write code that causes core-generated action links to refresh via AHAH, you will encounter this same problem.

The JavaScript in FBSS that attempts to fix this situation works as expected: it makes sure that the current URL is always the destination of FBSS action links. The reason there is a problem is because of a somewhat strange behavior in the i18n module that prepends the language code to all URLs without taking into account whether that code is already prepended; and the FBSS JavaScript in question doesn't check whether there is a language code that needs to be removed to accomodate this.

@tic2000, I understand that you are frustrated with this problem. However, before you go around making accusations and claims, I recommend that you first make sure you understand what you're talking about.

tic2000’s picture

I understand perfectly what I'm talking about.
Can you provide me steps to reproduce the issue you say I should have with AHAH? Because I submitted a status, the view got refreshed and the links were working perfectly without that javascript. The status comments are refreshed through AHAH and the links to edit/delete them are working.
I wrote a module using ctools to ajaxify the core comments. The links are working after the comments are added through ajax.
In none of those cases there is a need to rewrite the link through javascript.
It's not strange for you that FBSS is the module that does it differently although it's not the only module to use AHAH or refresh content through ajax?
If you take a second look in that issue you pointed us to, the problem was somewhere else, because he had a destination set as an absolute path, which doesn't happen normally. Even if the path would be wrong, it would still be a path like my-ahah-callback-path/something/bla/bla
http://d6.miidecuvinte.ro/en/user/1 here is a page with English using a prefix in a multilingual system. You can change en to ro if you want to see in another language. That javascript is commented out. I post and the destination is correct in the edit link, the ahah callback path is nowhere to be found.

So I reiterate my request for steps to replicate the issue you talk about and if it's a legitimate issue and not a missconfiguration or bad coding, I will try to come up with a fix.

Remember, Drupal is not babysitting bad coding or configurations and I think contributed modules shouldn't do it either, especially if they brake normal behavior, like it happens in this case.

icecreamyou’s picture

Can you provide me steps to reproduce the issue you say I should have with AHAH? Because I submitted a status, the view got refreshed and the links were working perfectly without that javascript. The status comments are refreshed through AHAH and the links to edit/delete them are working.

FBSS has some processing in PHP as a backup method that makes the links work correctly on user pages without the relevant JavaScript. To see the problem, remove the JavaScript in question, and then try for example putting the status update block on a node page, submitting a few statuses via AHAH, and then deleting one of those statuses. When you click Confirm to delete the status, you will be redirected to /facebook_status/js which will look like a completely white page with some gibberish at the top.

It's not strange for you that FBSS is the module that does it differently although it's not the only module to use AHAH or refresh content through ajax?

That is quite a bit of hyperbole. The only other module I know of that works like FBSS is Shoutbox, and it uses a method that is very similar to what FBSS does to fix links with destination redirects (but Shoutbox's method is even less robust). It's possible that there is a better way to solve this server-side, like using a different method to figure out the destination parameter, but I haven't discovered such a way. However, this is a problem that is reproducible in several of the very small number of modules that update content using AHAH.

If you take a second look in that issue you pointed us to, the problem was somewhere else...

Well, yeah, obviously. That link was a suggestion of one problem that the original poster could have had before it became clear what the problem was.

Remember, Drupal is not babysitting bad coding or configurations and I think contributed modules shouldn't do it either, especially if they brake normal behavior, like it happens in this case.

Remember that I have no obligation to babysit people who are aggressive, disdainful, or otherwise disrespectful in the issue queue. I appreciate that you care about this issue, and there are plenty of ways to communicate that without being critical. Believe me, I would love for this to be fixed. Just don't discount the fact that I've put a lot of work and research into making this module work for the vast majority of its several thousand users.

tic2000’s picture

I'm sorry, but still I can't replicate the issue.
I've put the form at http://d6.miidecuvinte.ro/en/node/3 and it works perfectly with that javascript commented out. No redirect to a page with a json text (aka gibberish). And that's because I never got a destination to be facebook_status/js on any of the links. It worked on the admin/build/block page too.
I tried this with legacy enabled and disabled.

I have another site where the form is on a special page (but I modified the form to use ctools instead of the drupal AHAH which allows me to update content in more than one place in the page) and that works perfectly without that javascript.

And I don't understand where do you get that aggressive feeling from me.

icecreamyou’s picture

Oh, I'm sorry, I forgot some things. It's been a long time since I've looked at the code in question, and I also haven't worked with the 2.x branch in awhile. Allow me to correct myself.

When I first wrote the offending code, FBSS updated the view of status updates on the AHAH callback page, so this fix was absolutely necessary. Since then, I have moved to a more flexible model that doesn't do this. Now, the only time the destination parameter rewriting is necessary is for sites that choose to show the latest status above the status update form, because that is still updated on the AHAH page.

So the first step of fixing this issue is to restrict the selector that finds links that need rewriting so that it only selects the ones that actually do need rewriting! The second step is to detect when there is a language prefix enabled, and to remove it if necessary (which is somewhat more complicated, but probably no harder than passing a variable from PHP to JS).

Additionally, the ability to have the last status update appear above the status update form no longer exists in the 3.x branch, so I believe the offending JS can be taken out completely.

I apologize for the misinformation; it appears that I am the one at fault here.

tic2000’s picture

Well, the issue for the content on top is that it changes the destination. Not to the ahah callback, but to share-status.
I found a solution, but it feels kind of hackish. My solution is to add $form['q'] as a hidden element in the form outside the replace div. Than you can read that from $form_state['post']['q'] in the form. If it's set it means the form was submitted through ajax. Than append the value to $status->q, or you could use a $_SESSION, or w/e, so you can read that value in the theme function that renders the slider. I used $status->q.
Than in the theme function check for that value and if it's set give $q its value. Only if it's not set go through the normal code of giving the $_GET['q'] and checking if it equals facebook_status/js.

I'm sure there are other ways too, but I'm to tired to think at anything else right now.

icecreamyou’s picture

Yeah there is PHP code that detects if the current page is the AHAH callback page and changes the destination to share-status if necessary just so people don't end up on a blank white page.

My strong preference is to solve this in JS, and it can definitely be done. What needs to happen is that hook_init() needs to check if a language code is in use, and if it is, that code needs to be passed as a JS variable. Then the destination-modifying code can just chop off that code from the beginning of the paths it constructs.

tic2000’s picture

I think that trying to use js to fix this is more prone to errors.

icecreamyou’s picture

Status: Active » Needs review

Just committed an attempt at fixing this to dev. Please test it and see if it resolves the issue for you. I am unable to test it (for some reason I can't get language domain prefixes working on my testing setup) and I may not have gotten it exactly right (I had to guess on whether to include slashes in the language prefix). FWIW, this is a JavaScript-based solution.

I would appreciate if anyone can test this in the following situations:

  • Language prefixes off and Clean URLs off
  • Language prefixes off and Clean URLs on
  • Language prefixes on and Clean URLs off
  • Language prefixes on and Clean URLs on
  • Language prefixes on and Clean URLs on, on a website that is not in the webroot
tic2000’s picture

Status: Needs review » Needs work

Doesn't work.
After a status is submitted the links still sets the destination to "share-status".
The links in the view do point to the correct path.

Without clean url on the destination for the view links becomes "node" instead of "node/3". The one in the slider points to the same share-status.

icecreamyou’s picture

Okay. I have decided that the best way to fix this is to just move the slider out of the form and refresh it the same way that views are refreshed, rather than trying to mess with the JS. Patches appreciated. This is basically the last thing I want to do before releasing 6.x-2.4, which should be the last release in the 2.x branch.

icecreamyou’s picture

Status: Needs work » Fixed

Alright, committed the fix mentioned in #25 to dev. Please confirm if it works (only one person needs to do this). I'm going to wait a week to make sure this didn't create any problems and then tag the 6.x-2.4 release.

Status: Fixed » Closed (fixed)

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