Support from Acquia helps fund testing for Drupal Acquia logo

Comments

GaëlG’s picture

Title: Title module prevents term label localization » Title module overrides label callback (prevents term label localization)
Version: 7.x-1.0-alpha4 » 7.x-1.x-dev
Status: Active » Needs review
FileSize
1.23 KB

Title and i18n taxonomy modules both use hook_entity_info_alter() to change the label callback. Here's a way to fix the problem.

Status: Needs review » Needs work

The last submitted patch, i18n_taxonomy_compatibility-1865604-1.patch, failed testing.

plach’s picture

Status: Needs work » Needs review
plach’s picture

Wouldn't it be simpler to just skip registering the label callback if there is already one defined?

Status: Needs review » Needs work

The last submitted patch, i18n_taxonomy_compatibility-1865604-1.patch, failed testing.

GaëlG’s picture

It's simpler but I'm not sure it fits every context. It would make Title unavailable for any bundle already having a label callback.

plach’s picture

Sorry, I missed that the patch is calling the fallback function only when fields are not replaced.

Anyway, I'm not sure about generalizing this approach: what if another module had the same need? Should it define a 'label fallback fallback' key?

I don't feel strong about this, but perhaps I'd prefer something dealing explictly with i18n. Related issue: #1661348: I18n Taxonomy integration.

GaëlG’s picture

Yes, I wondered the same. That's why I said it's a way to fix it, among others. It may not be the best. In any case, Title should not alter the way the label is generated when default title field is not overridden. This is true for i18n_taxonomy, but potentially for other modules too.
At least Title should do more checks before it changes the label callback, to do it only if really needed.
Decreasing module weight might be another way? So that i18n_taxonomy_entity_info_alter() is called after title_entity_info_alter()?
Actually, it looks like this kind of "hook competition" problem can only be solved on an individual case basis.

plach’s picture

Actually, it looks like this kind of "hook competition" problem can only be solved on an individual case basis.

Yes, I am afraid we cannot escape from a certain level of "structural" incompatibility. Anyway you solution might be good after all if we "standardized" it somehow:

+++ b/title.module
@@ -69,6 +69,10 @@ function title_entity_info_alter(&$info) {
+          if (isset($info[$entity_type]['label callback'])) {
+            $info[$entity_type]['label fallback'] = $info[$entity_type]['label callback'];

We could have:

$info[$entity_type]['label fallback']['title'] = $info[$entity_type]['label callback'];

This way we could have a sort of label callback chain where every element would be responsible for a single fallback. Btw, Title is very likely to be the last module altering entity info since it has a very low weight and it does http://drupalcode.org/project/title.git/blob/refs/heads/7.x-1.x:/title.m....

Minor:

+++ b/title.module
@@ -69,6 +69,10 @@ function title_entity_info_alter(&$info) {
+          // Store the original label callback for compatibility reasons (i18n_taxonomy etc.).

Let's skip the example to fit into the 80 chars :)

Not sure why tests are failing.

GaëlG’s picture

Status: Needs work » Needs review
FileSize
1.25 KB

All right. Like this?

plach’s picture

Status: Needs review » Fixed
FileSize
1.22 KB

Committed and pushed with minor changes, thanks.

Status: Fixed » Closed (fixed)

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

  • Commit 54828ae on 7.x-1.x, workbench authored by GaëlG, committed by plach:
    Issue #1865604 by GaëlG, plach: Fixed Title module overrides label...