modules/fb support other languages and locales

Better to patch fb.module. This should be built in, I just wasn't sure how to implement it.

Please feel free to assign this issue to yourself. Submit a patch when you have one and we'll review it.

Just looking it over...

It looks like the call to $_fb->api('me'); could happen on every page. If I'm reading that right, that's way to often. Calls to $_fb->api() could contact facebook over http, so are very expensive. It needs to be cached, maybe in $_SESSION?

Also that call should be wrapped in try {...} catch {...}.

With those fixes, an awesome feature!

I applied this patch and the language changes according to the user settings in facebook.
Nice work, thanks!

Any update on this? I applied the patch and is working fine.
Is this going to be applied to the deve version?

Thanks!

The patch seems to work as described. In fact, it seems to work to well. After patching, the Language Switcher Dropdown no longer is able to switch to another language. It is automatically reset to the user's default language. This may be a result of what seems to be a DFF language related bug, so I'll first try to find a solution to that (heads up, Dave, new issue in the making), unless anyone else is experiencing similar problems.

Patch n°8 is working very nice for me :-)

Patch is working fine for me too, for the Like button. However, the "Connect with Facebook" button is still in English. And btw, why does it say "Connect with Facebook" and not "Login with Facebook", as on the Facebook Single Sign-on developer page?

I did a string replacement and then realized that in fb.module, this text is hardcoded (line 1124 in my patched version), as well as line 169 & 173 in fb_connect.module. However, it is t() wrapped on line 571 in fb_user.module.

Are all those instances supposed to be translatable? Or should this script be enough to get in the local language? <script src="http://connect.facebook.net/en_US/all.js"></script>

subscribing....

So looking this over, several things...

1) I wonder if this can be placed entirely in its own module. I suspect there will be more language/locale features needed and it would be nice if they live in one place. (possibly fb.module will have to make it easier to change js settings)

2) This adds a call to $_fb->api('me') to every session. This is undesireable, and there's already a call made every session in fb_api_check_session(). I think fb_api_check_session() should call $_fb->api('me') instead of users.isAppUser (assuming $_fb->api('me') will fail if the session is not valid), and somehow make the result available to this code. In other words call into facebook just once per session instead of twice, as this patch currently does.

3) This patch sets $fb_settings['js_sdk_url'] twice which is a little confusing.

I think this is worth including in the 3.0 release. Anyone want to make those adjustments?

hi there. i'd like to benefit from this change, so therefore would like to offer my efforts in moving it forwards. i havent done anything with drupal CVS before, but i just watched a drupal CVS video from 2007 (it helped a bit, but was also a little confusing in places) and did some reading and am going to try to check out the fb module in my eclipse environment. i am going to 'hack' my site's version of FB to get it working in french and spanish for now, but then i'd like to help finish off this issue as best i can.

sorry if this is a stupid question, but i'm struggling to apply the patch. I'm using Eclipse. I grabbed the fb code. I wasnt sure what to take - at first i figured it would be a v6 branch, but after grabbing one and suspecting it was old i decided to just take HEAD instead - is this sensible for this project?. Also, I guess the patch created in #8 above may now be a little hard to apply as the main code has moved on no? I try to apply the patch (in Eclipse -> Team) and it seems to only have a slight problem which I think I can fix up manually. Just a bit worried that I may have the wrong code branch and be doing something daft trying to apply the patch to it.

Drupal.org has several help showing just how to create and apply patches. The examples use cvs from the command line and it's well worth learning how to do that. IDEs like Eclipse which make things "easier" really make them more opaque, so you don't even know what the underlying error is. At least for me who doesn't use eclipse, maybe someone out there could tell you exactly what's wrong.

And yes, version 6.x-3.x-dev is the CVS HEAD. You can learn that sort of thing by going to the project page and clicking "CVS Instructions" or "browse the CVS repository". The patch program is pretty smart so I'd expect most if not all of the patch to apply even as the code changes.

yeah, it seems fine I think. It struggled a little bit because I think things had moved on a little from when the original patch was done. (i.e. the same line was changed in the patch and since then in the code). I suspect that would have happened at the command line too. I think it is fine. I'll try to get into the CLI too, but I have been using Eclipse for 7 or 8 years (Java, PHP and Flash/Flex programming), so I kinda like my IDE.

Now I have the patch applied, I will take a look at your comments in #16 and try to make sense of them and see if I can make an improvement on the patch. I'm in Spain and it is getting near the end of my working day here, so I'll probably be picking it up tomorrow. The basic patch seems to work fine, though. Looks like a few people will appreciate having this in a future release.

I consider this worth including in the 3.0 release (which I'd live to have done in the next 2-3 weeks).

As I said in #16, I think it is best to move this code to it's own module (maybe fb_i18n.module). For that two happen, I expect 2 changes to fb.module will need to happen.

1) The fb_api_check_session will change or be replaced by something useful. Something that learns the locale in the way Anclywen's patch learns it.
2) The logic around js_settings needs to change. Right now all Drupal.settings.fb... values are set in fb_init(). It would be better to a) delay the javascript settings until later in the request handling, and b) allow other modules (fb_i18n among them) some way to alter the settings. I've noticed that og.module uses og_preprocess_page() as a way to delay the javascript settings until later, and I suspect that's worth emulating.

This issue is assigned to Anclywen and AndyThornton has also offered some help. Are you guys up for taking this on and submitting patches? Personally I don't have multilingual apps yet, so I haven't had the itch.

Hey Dave,

Anclywen hasnt posted on this post since August, but I am happy to take whatever has already been generously done and try to finish it up for the 3.0 release. Sorry in advance if it take me a bit of time to figure things out and if I ask silly questions. Would rather make sure I understand what I am doing rather than making you review a crappy patch.

I am using the HEAD version from yesterday (i state that because the version of fb_api_check_session I have still doesnt yet called api ('me') - wasnt sure from #16 above).

It is not clear to me why Anclywen potentially sets the js_sdk_url twice. I think it is because the first call to _fb_language_default (before global $_fb is set) will return a locale that is not based on their facebook locale settings . I think we could set the js_sdk_url once a little further down in the function and modify _fb_language_default to respect the 'override' flag. However, that function also gets used in another area of the code, so we'd have to make sure that would continue to

Some of this is a little moot, though, as you are suggesting moving this into a different module and using the hook_preprocesss_page call too. Do you think that the fb_i18n.module should be a high-level module within the fb project? There are already quite a few modules people have to consider enabling or not in the fb project, perhaps this functionality should just be part of the core functionality (in a fb_i18n.inc file)? No? We could expose a hook that would allow other modules to provide the locale information, and it defaults to the behaviour Anclywen has coded if not?

Also, regarding hook_preprocesss_page - i think that method would presuppose that the site is using the phptemplate engine. I'm no drupal expert (at all), so I could be wrong, but is that something you'd like the fb module to assume (mind you, dont a lot of drupal sites have phptemplate engine themes)? I dunno if that is already implicit in some of the other stuff you have done? I'm happy to take your lead on this. If you want me to move the code into fb_preprocesss_page I will. Would it be pretty much all of the contents of fb_init? Looks like most of it is for setting js stuff.

Cheers

p.s. i am on a few of the drupal irc channels (AndyThornton) if you feel irc would be a better form than this. Up to you.

I haven't made any change to fb_api_session_check. All the things I mentioned in comment #21 are to be done.

Regarding the extra module, I don't really care if it's fb_i18n.module or fb_i18n.inc. (If it works as an .inc) I have a suspicion that as time goes on there may be more i18n-specific features to add, and I don't see another module as a bad thing.

I don't think hook_preprocess_page has anything to do with phptemplate, but please correct me if I'm wrong. I think it's part of core theming.

I want a mechanism that makes it easy for other modules to alter the javascript settings, because this isn't the only case where that would be helpful. Other cases included setting the js_sdk_url to be local (you don't have to use facebook's copy and it's easier to debug against a local copy), changing the reload_url, or who knows what else that I haven't thought of. And I'd like that mechanism to be toward the end of page processing, rather than in hook_init() where all the settings are now. I suppose I'd consider a patch for this particular issue even if it didn't implement hook_preprocess_template.

hey dave,

eek, I hope I did this right. So I have taken Anclywen's patched and attempted to move it forwards a little. I have a couple of questions, so I dont expect this to be the final version, but I just wanted to check I am heading in the right direction.

I switch the name of the include file to be fb_i18n.inc. Other modules can extend the JS setting by implementing hook_fb_js_settings and returning an associative array of settings. fb_i18n implements that hook and returns the locale aware js_sdk_url setting.

I moved the call to api('me') to a function called fb_get_facebook_user_object. It stores the result of the call in the drupal session. I haven't yet changed fb_api_check_session, as I wasn't quite sure how that needs to change.

Also, I haven't moved the code to fb_preprocess_page yet. More than happy to do it. I was a little unclear, though, of how much has to stay in fb_init and how much is safe to more to fb_preprocess_page. Naively, I was tempted to just rename the function - but I balked on that.

Anyway, I hope I am on the right track, and I hope I bundled the patch successfully (first time for that too!).

Cheers,
Andy

Andy,

Thanks for keeping up with this. Here's some feedback on the patch (don't take it the wrong way)

The big functions _fb_i18n_default and _fb_i18n_list are only needed for the admin form, correct? If so let's move them to fb.admin.inc. And not load all that during fb_init.

I don't want to introduce a new hook_fb_settings. There's already a hook_fb, and a few other hooks each of which follows the same signature (passing useful info like $data['fb'] and $data['fb_app'] to the modules which implement the hooks. But in this case I don't think we need a new hook invocation. This is part of why I want to move the javascript to the page preprocessing... third party modules may decide late in the page request what the settings are. In this case it doesn't matter so much, but take the reload_url as an example; there are cases where other modules want to change that, but well after the hook_init has completed.

So for these settings it would be nice to make a routine with a static cache called say fb_js_setting($key, $value) which just stores the values, until the last possible moment when it is used to write the javascript. If the preprocess_page stuff is too complicated, just do all that during hook_init and I'll figure out a way to move it somewhere else. At this point I plan to move only the drupal_add_js away from fb_init and into the hook_preprocess_page.

Regarding the $fb->('me') data in session... I'm not sure if there's a performance hit for storing extra data in session. I'd prefer to store nothing in the session, but I'm not sure there's any way around it. I'm curious how does the i18n module determine the user's locale? does that value ever differ from the value we learn from facebook?

I'm not too concerned about the fb.i18n.inc either. I was thinking all of this could be moved to it's own module. But if we don't go that route, let's just get it working and I may reorganize the files at some later date. But I do feel those big routines should be in the admin.inc file, so less is loaded during most page requests.

The change to fb_api_check_session is simply to to call $fb->('me') instead of what it calls now. It doesn't matter so much what is called there, just whether or not an exception is thrown.

And here are some really minor points... When coding for drupal, don't delete the trailing commas at the end of an array entry. (not deleting makes patches easier to read) And Assign this issue to yourself if you're still up for it, which I appreciate.

That's all I can think of looking this over!

Hi Dave,

Cheers for the feedback. Sorry for the delay again. I promise the turnaround next time will be quicker. I hope this next patch brings us closer to what you'd be happy with. I dont mind receiving further feedback, providing you dont mind giving it. I'm keen to see it done right, rather than half-assed/rushed.

"The big functions _fb_i18n_default and _fb_i18n_list are only needed for the admin form, correct?" - NO. The former function is used to get the locale of the user, the latter is only used by the admin UI, though, so I have moved that. Now we just have one function in the fb_i18n.inc file - starting to wonder if it has much utility as a separate file - maybe it should just be collapsed into fb.module and stripped out if and when more i18n is needed? Anyway, that's up to you - I've left it as an include for now.

I've removed the hook, and added a fb_js_setting function instead. I've moved the adding of the js settings into the fb_preprocess_page function too.

Do you think there is a better way to deal with the user object checking stuff? At the moment we have the requirement of needing the FB locale and also just knowing whether an exception was thrown. Given we want to make the call only once, the minimum we could do to achieve both goals is to only cache the FB locale (and probably a connection success variable, just in case it did connect, but didnt have a locale - not sure if that is possible with FB). However, I figured we may as well cache the 'parent' object in case we needed to use something else in the future. Not sure how many extra bytes of memory that will take, though (mind you, chances are the facebook PHP library module may keep that memory around anyway).

Anyway, let me know what you think? As I said, I'll turn around any further comments quicker next time.

The patch did not apply cleanly for me, but I am looking it over.

I'd like to keep that big array in fb.admin.inc. It seems it's only needed for the elseif clause:

  //Check previously assigned locale                                                                             
  if ($default_language = variable_get('fb_language_'. $lang_code,  FALSE)) {
    return $default_language;
  }
  //not assigned                                                                                                 
  elseif (isset($languages_map[$lang_code])) {
    return $languages_map[$lang_code];
  }

is that elseif clause even needed? Won't the variables be set?

I've reworked it a bit. The way I see it, we can use the language from facebook if known, otherwise choose only among the configured drupal languages. Let me know if I misunderstand.

Also, my setup has only english, so I haven't really tested this. Hopefully though it will apply and work.

Another nice-to-have would be to set the local account's locale from what we learned from facebook, when fb_user.module creates the account. But every additional feature like that makes me think this belongs in its own fb_i18n.module.

I couldnt apply the patch you gave me back either. I figured maybe my code was out of sync with the head revisions so I did a CVS update, but it wont apply against the latest code. You got rid of i18n.inc, right? Isnt in the patch.

Yeah, I got rid of it. And technically that patch is against my svn repo which might be slightly different from the CVS (they are mostly in sync but not always). Did you use `patch -p0`?

hey dave,

i'll need to figure out what that -p0 setting is, against your advice i've been applying the patches through my Eclipse. what are you plans for a CVS delivery? just wondering if there is anything more to do on this. last posting you mentioned you've reworked thing a bit. maybe i can just grab in the next time you do a dev check-in and test it out then? if there are any tweaks you'd like me to do i could do them too at the same time? if this doesnt work i can try to apply the patch again - just a bit nervous i might end up with things in a funky state.

cheers,
Andy

Probably the patch doesn't apply because you're working with your already patched copy. If you got a clean version from CVS, my patch should apply.

From the command line I would apply a patch with

cd modules/fb
patch -p0 < /tmp/fb_i18n.diff

(assuming I had downloaded the above patch to /tmp/fb_i18n.diff)

I may just check in the change, but normally I wouldn't do that, not knowing whether something works or not.

OK, I went ahead and checked in my changes. So a cvs update will get them (and after a while, a download from d.o).

I'd still like some review, because I've only been testing on a single-language site.

This patch is already in 6.x-3.0-rc1
Thanks

I consider it fixed unless someone points out that it doesn't work.

Sorry Dave, but it doesn't work for me.
In Drupal, I have two lang (ENG + ITA) installed, but only one enabled (ITA).
Obviously, i set ITA as default language

In "admin/build/fb/languages" i have checked "Use locale learned from facebook, if available.".
If i'm logged in Facebook (you guess? Italian :-D ), but js variable Drupal.settings.fb.js_sdk_url is always "http://connect.facebook.net/en_US/all.js" (while i need "http://connect.facebook.net/it_IT/all.js")

How can i help to trace down the problem?
I tried while logged in or logged out, but it's the same.

NB. I have cacherouter module with memcache backend. Is this a problem?

Thanks

I think "needs work" status is more appropriate.

Another side note (sorry, but it's late here in italy :-D )
Patch in comment #8 worked well with a dev release. But i can't remember now how long ago the dev was.

Thanks again

Please go to Administer › Site building › Facebook Applications › languages. Check the settings and submit the form. Then, does it work?

Ping. Does saving that form change anything?

This is maked "needs work" but I'm not sure what the problem is with what's currently checked in.

subscribing

#1162984: Canvas language path prefixes
Marking this as dup, because other thread has patches and more recent comments.
I believe at this point, regular pages work in multiple languages, but form submits have problems.