The D7 i18n taxonomy sub module has been renamed to i18n_taxonomy (was i18ntaxonmy). The APi has beenchanged as well. One place I found that has to be updated is the following:

old:

function _hs_taxonomy_hierarchical_select_terms_to_options($terms) {
  $options = array();
  foreach ($terms as $key => $term) {
    // Use the translated term when available!
    if (module_exists('i18ntaxonomy') && isset($term->vid) && i18ntaxonomy_vocabulary($term->vid) == I18N_TAXONOMY_LOCALIZE) {
      $options[$term->tid] = tt("taxonomy:term:$term->tid:name", $term->name);
    }
    else {
      $options[$term->tid] = t($term->name);
    }
  }
  return $options;
}

new:

function _hs_taxonomy_hierarchical_select_terms_to_options($terms) {
  $options = array();
  foreach ($terms as $key => $term) {
    // Use the translated term when available!
    if (module_exists('i18n_taxonomy')) {
      $options[$term->tid] = i18n_taxonomy_term_name($term);
    }
    else {
      $options[$term->tid] = t($term->name);
    }
  }
  return $options;
}

This works fine on first load. However, on ajax callbacks the i18n taxonomy module seems to be unable to retrieve the language correctly (the global langauge object is set to the site default). So I think the language should be passed to this function, so it can be passed through to function i18n_taxonomy_term_name() (and t() for that matter):

function _hs_taxonomy_hierarchical_select_terms_to_options($terms, $langcode = NULL) {
  $options = array();
  foreach ($terms as $key => $term) {
    // Use the translated term when available!
    if (module_exists('i18n_taxonomy')) {
      $options[$term->tid] = i18n_taxonomy_term_name($term, $langcode);
    }
    else {
      $options[$term->tid] = t($term->name, array(), array('langcode' => $langcode));
    }
  }
  return $options;
}

However, this is only part of the solution. I have no idea how to get the correct language in ajax callbacks, and why it is not known in the first place. I guess each ajax callback should know the language of the page the requests come from???!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fietserwin’s picture

Status: Active » Needs review
FileSize
13.38 KB

I found some information about this on http://www.using-jquery.com/2011/05/ajax-with-drupal-callbacks/. So I changed the code in the following places:

file: hierarchical_select.module

function _hs_process_attach_css_js($element, $hsid, &$form_state, $complete_form) {
  ...
             'ajax_path'        => url('hierarchical_select_ajax/' . implode('/', $element['#array_parents']) . '/' . $complete_form['form_build_id']['#value']),
  ...
}

and subsequently
file: hierarchical_select.js, line 531

  var url = Drupal.settings.HierarchicalSelect.settings[hsid].ajax_path;

This now works fine on my system, thus passing in the language, is no longer necessary. Attached patch contains a lot of white space removals as well, so it may look quite long, but the real changes are just in 4 lines.

EDIT: oops, the patch also contains my patch for #1136370: Notice: Undefined index: exclude_tid in hs_taxonomy_hierarchical_select_children()

Wim Leers’s picture

Title: Adapt code to changed name and API of i18n_taxonomy module » D7 i18n Taxonomy compatibility
Category: bug » feature
Status: Needs review » Fixed

Please try to post patches without whitespace changes next time :) I don't mind applying the patch manually because you've already contributed so many good changes! :) (For which you of course have my gratitude.) It also seems that the language changes (using url() in hierarchical_select.module and in the .js code) were actually *reversed* in the patch: instead of your changes being added, they were actually being removed. Something strange must have happened there.

Since ajax_path no longer contains the path to the AJAX callback, but an entire URL, I renamed it to ajax_url as well.

But I applied all changes manually, so now it should be working correctly. I've committed the patch (http://drupalcode.org/project/hierarchical_select.git/commit/e8241a7) — please report back if it's all working correctly or not.

P.S.: only just now, I learned about proper authorship attribution through git (http://drupal.org/node/1146430) — I had no idea this existed, after years of committing the way we used to do when using CVS. My apologies. You should have had many more commits on your name!

fietserwin’s picture

I make most patches in projects where I am actually working on (and where I need the fix and can test it). In these cases, I'm a bit reluctant to use git and work with HEAD (though I do use dev versions). So I often make my patches manually using WinMerge (yes I am on Windows ....) and thus sometimes I create the reversed patch. And this white space removal is a setting in Eclipse, And I am often too lazy to remove it from the patch and, IMHO, it shouldn't have been there anyway :)

As far as attribution to git commits concerns, I'm not interested. I think for core it is a nice option. Attribution in the changelog is different, that is not more than fair, though I myself tend to not attribute mere reporters if they don't at least propose a solution as well.

Wim Leers’s picture

Actually, your patch was *adding* whitespace… Whitespace should indeed not be there, I agree with that :) But even if it just removes whitespace, it makes the patch much harder to review. Posting two separate patches makes much more sense then: one with whitespace fixes, one with real/semantical changes. This is easy with a good git UI, such as Tower (http://www.git-tower.com/).

Regarding attribution: well, I'm relieved that it's not a big deal for you, but I'll make sure to do it correctly from now on ;) :)

In any case: thank you very much for your efforts, and please keep the patches coming! :)

Wim Leers’s picture

Status: Fixed » Active

Apparently, there are still problems: http://drupal.org/node/1068366#comment-4815068

dgastudio’s picture

trying to edit field gadget options.
Fatal error: Call to undefined function tt() in /home/*****/sites/all/modules/hierarchical_select/modules/hs_taxonomy.module on line 939

i'm using dev version of i18n (2011-Jul-28) and latest HS dev version.

taxonomy translation is enabled.

upd: i have create a new issue in i18n thread.

http://drupal.org/node/1237664

fietserwin’s picture

Status: Active » Needs review
FileSize
683 bytes
2.43 KB

Apparently the changes in the taxonomy part got lost. I created a patch for that part against the aug 2 dev version to fix that. Note that it also includes the optimization of #1220830: Minor performance improvement for _hs_taxonomy_hierarchical_select_terms_to_options() as that touches the same code and is being hold up because of the same confusion: tt versus using the D7 api.

I included a patch without white space removal (72) and one with (71). So you can look at the real changes (in 72) and have the white space removed by subsequently applying 71 :)

dgastudio’s picture

everthing works fine now! thank u!

pfrenssen’s picture

Fietserwin, thanks for the fix!

The tt() function is however also used in hs_taxonomy_hierarchical_select_item_get_label(). I have attached a patch which follows your example.

Wim Leers’s picture

Status: Needs review » Fixed

Thanks, committed: http://drupalcode.org/project/hierarchical_select.git/commit/da3b12e

P.S.: I used the command git commit --author="fietserwin <fietserwin@750928.no-reply.drupal.org>" --author="pfrenssen <pfrenssen@382067.no-reply.drupal.org>", but apparently git only supports one author. So your attribution was lost, fietserwin. Next time, I'll just do two commits, then.

fietserwin’s picture

Status: Fixed » Needs work

I stumbled upon another module that didn't translate taxonomy terms and the maintainer over there thought the t() fallback if i18n_taxonomy is not enabled is a bad idea:
- In an English only install it is a wast of time
- In a single lingual non-English site it might inadvertently translate terms that are defined in that language (if it happens to exists as an English word/phrase and it appears in the translation tables)
- The taxonomy module itself does not translate terms, leaving this task to i18n. Thus using t() in a contrib module seems counter productive. You either use i18n or you don't translate your terms.

See issue: #1264264: Terms are not translated

Gábor Hojtsy’s picture

Yes, you should either use i18n or entity_translation module to translate taxonomy terms. t() for user provided data like taxonomy terms is a pretty bad idea. It assumes the source is English, does not have change management, if you fix a typo in your original language, it looses all translations immediately, it keeps unused translations around, etc.

Berdir’s picture

guillaumev’s picture

Please see the patch I posted here: http://drupal.org/node/1563890#comment-6150054. It might not fix all of the issues in this case, but it might fix some of them...

Gold’s picture

Status: Needs work » Fixed

Tidying things up. It's been 6 years since the patch was committed and no new patches since then. Setting this back to fixed.

If people think this still needs to be addressed please open a new issue with a patch.

Gold’s picture

Status: Fixed » Closed (fixed)