Reproducing the bug:

  1. Set the widget to Hierarchical Select (hs_taxonomy) for a term reference field.
  2. A vocabulary where the term name is replaced by name_field provided by Title module.
  3. The terms are translated in other languages than English.
  4. On a non-English page, the HS widget displays untranslated terms in the root level select.

This occurs due to the fact that _hs_taxonomy_hierarchical_select_get_tree() doesn't perform a full entity load of taxonomy terms. The original function (taxonomy_get_tree()) has such option.

CommentFileSizeAuthor
#11 hs-title-2252949-11.patch702 bytesbogdan.racz
#1 hs-title-2252949-1.patch1.79 KBclaudiu.cristea

Comments

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Active » Needs review
StatusFileSize
new hs-title-2252949-1.patch1.79 KB

Here's a patch.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Issue summary: View changes
kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

Works for me, thanks!
I'm not sure if we want code for another module inside HS, although other modules do it too...

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented

HS already tests the existence of 3rd party i18n_taxonomy. This is not a bad practice especially for popular modules. And Title is one of them.

If you are happy with the fix and it does the job why not set this issue to RTBC?

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Needs review » Reviewed & tested by the community

I was holding off on setting this as RTBC until another party confirmed it works, but seeing as it works and it's clearly documented...

Off topic:
About using module_exists(): I tend to avoid it when writing modules or patches because:

  • you're taking up the responsibility to keep track of code changes in all modules you use module_exists() for
  • you're privileging some modules over others
  • you may as well expose your data to any module by providing hooks

I'm not saying you shouldn't use module_exists(), I'm just stating that it isn't always the best solution.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented

I'm not saying you shouldn't use module_exists(), I'm just stating that it isn't always the best solution.

Well, I agree but I considered also next pros:

  1. The official development branch of Drupal is 8.x. In Drupal 8, Entity Translation is part of the core. Title is only an application of Entity Translation. It seems natural for me to consider ET & Title more than simple contrib modules even if we are discussing about HS for D7. So, yes, ET & Title are somehow privileged modules right now.
  2. It would have been very simple for me to avoid that if (module_exists()) statement and to draw a simpler patch. But why overloading with a full entity load outside the use of Title module, when there's no need for that. Performance was the main reason.

  • stefan.r committed c4579a7 on 7.x-3.x 
    Issue #2252949 by claudiu.cristea: Fixed Untranslated root level terms.
stefan.r’s picture

stefan.r commented
Status: Reviewed & tested by the community » Closed (fixed)

Committed, thanks!

stefan.r’s picture

stefan.r commented
Status: Closed (fixed) » Fixed

Status: Fixed » Closed (fixed)

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

bogdan.racz’s picture

bogdan.racz commented
StatusFileSize
new hs-title-2252949-11.patch702 bytes

I've noticed that in 7.x-3.0-beta2 the $term_entities variable is not taken into consideration anymore.
It was removed in this commit: https://www.drupal.org/node/544324#comment-9895657

Anyhow, I have re-rolled the initial patch to the current version, and it seems to work again.
Please review.

bogdan.racz’s picture

bogdan.racz commented
Status: Closed (fixed) » Needs review
sylus’s picture

sylus commented
Status: Needs review » Reviewed & tested by the community

I think we should have filed a new issue since original closed but since here and patch works as advertised.

  • stefan.r committed 720077b on 7.x-3.x authored by rbmboogie 
    Issue #2252949 by claudiu.cristea, rbmboogie: Untranslated root level...
stefan.r’s picture

stefan.r commented
Status: Reviewed & tested by the community » Fixed

Committed, thanks!

Status: Fixed » Closed (fixed)

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