Core is going to introduce a new function to deal with entity language in a generic fashion:

#1495648: Introduce entity language support

By leveraging it we don't need to depend on the Entity module anymore.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plach’s picture

Status: Active » Needs review
FileSize
4.56 KB

Status: Needs review » Needs work

The last submitted patch, title-1519930-1.patch, failed testing.

plach’s picture

Status: Needs work » Needs review

This needs to core issue to be committed to be tested (or a applying the patch patch for manual testing).

fabsor’s picture

I tested this in NodeStream which makes heavy use of entity translation and title, and everything works perfectly, even for terms which is currently mostly broken without this patch and the one for core.

The patch is simple enough, I don't see how it would cause any harm. I will gladly RTBC this once the core patch lands =)

liquidcms’s picture

i'll need to try it on a fresh vocab as it doesn't work with an existing one. closer though.

i can edit and save both FR and EN versions of my term title but the value still isn't used anywhere. will test on a newly created vocab tomorrow.

liquidcms’s picture

hmm.. but perhaps the core patch is also required?

fabsor’s picture

@liquidcms the core patch is required as well to make this work.

plach’s picture

You will need to use the ml_edit_form branch of entity translation until the core patch is committed...

Edit: If you are not familiar with Git you can visit the link above and click on the snapshot link.

steinmb’s picture

FileSize
4.57 KB

Rerolling against latest dev.

Status: Needs review » Needs work

The last submitted patch, title-1519930-9.patch, failed testing.

steinmb’s picture

I'll keep testing this. It seems to work, but I keep getting warnings when saving terms.

Test setup

Notice: Undefined index: value in taxonomy_form_term_submit_build_taxonomy_term() (line 862 of /drupal/modules/taxonomy/taxonomy.admin.inc).
Notice: Undefined index: format in taxonomy_form_term_submit_build_taxonomy_term() (line 863 of /drupal/modules/taxonomy/taxonomy.admin.inc).
Pisco’s picture

I'm very confused. I got here from #1586002: Entity language handling broken and applied #9, but not #1495648: Introduce entity language support!
#9 seems to fix taxonomy term name translation with Drupal core 7.14, Entity Translation 7.x-1.0-alpha1 and Title 7.x-1.0-alpha2. Should I mark this to RTBC?

plach’s picture

This patch uses ET when it is enabled instead of the function introduced by the core patch, that's why this is working for you even without it.

plach’s picture

Status: Needs work » Needs review
FileSize
5.48 KB

The latest patch didn't play well with the new Entity Translation UI (ml_edit_form), I don't know why I did not spot this before. Please confirm this is still ok for you.

Status: Needs review » Needs work

The last submitted patch, title-1519930-14.patch, failed testing.

plach’s picture

Status: Needs work » Needs review
Pisco’s picture

As I already said, this patch fixes Title module when running with Entity Translation. As I tested the patch only with Entity Translation, I don't really want to set the status to RTBC. Has anyone tested this with the new Entity Translation UI (ml_edit_form).

I'd like to see the priority of this issue being elevated to major or critical as the module is currently broken.

plach’s picture

Priority: Normal » Critical

@Pisco:

Did you test also the latest patch?

I'd like to see the priority of this issue being elevated to major or critical as the module is currently broken.

Makes sense.

Pisco’s picture

@plach yes, I tested #14 on a not too small installation with migrate, entity translation, views, panels, searchapi, facetapi … and everything seems to work fine.

Pisco’s picture

I retested this.

Test setup 1

  • Drupal 7.14
  • Entity Translation 7.x-1.0-alpha1
  • Title 7.x-1.0-alpha2 with patch from #14

Result: works perfect!

Test setup 2

Result: has some bugs.

If I create a node with language set to English, the title get's lost on save. If I then edit the node, write a new title and save again, everything works fine.

If I create a node with language set to Language Neutral everything works fine. If I edit that node and set the language to English, I get the following error upon saving:
Notice: Undefined offset: 0 in title_field_text_sync_get() (line 116 of …/sites/all/modules/title/title.core.inc).

My Conclusion

I see two separate and independent things here. One is Title module being broken when using with official releases. The other is, as the title says, preparing Title to use the upcoming entity_language() function and integrate with the upcoming ml_edit_form-branch from Entity Translation.

Neither #1495648: Introduce entity language support nor Entity Translation's ml_edit_form-branch have been released. The Title module is broken when used with the current release. I'd recommend publishing a new alpha release of Title with #14 included. This should make Title usable for users going with official module releases. Maybe we should create a separate issue for that?

On the other side and independently, we should continue improving the patch #14 to make it work with Entity Translation's ml_edit_form-branch and #1495648: Introduce entity language support.

What do you think?

plach’s picture

Status: Needs work » Needs review
FileSize
6.07 KB

@Pisco:

Great work!

I reopened #1586002: Entity language handling broken to fix the bug as you are suggesting. The attached patch includes the fix from there, please test it again. It would be great if you could test everything also without ET to ensure this works also for untranslated entitities.

Priority: Critical » Major
Status: Needs review » Needs work

The last submitted patch, title-1519930-21.patch, failed testing.

plach’s picture

plach’s picture

FileSize
922 bytes

This is what remains of the patch after splitting out the bug fix. #1586002-17: Entity language handling broken needs to be applied.

Status: Needs review » Needs work

The last submitted patch, title-1519930-24.patch, failed testing.

plach’s picture

Status: Needs work » Needs review
Pisco’s picture

Test setup:

I tested with and without Entity Translation as suggested in #21, although testing without Entity Translation is somewhat limited because I can only set a language on an entity, but not translate it.

Everything seemed work smoothly until I created a article with the language set to German. I then edited it and changed the language to English, that's when I got this error:
Notice: Undefined offset: 0 in title_field_text_sync_get() (line 116 of …/sites/all/modules/title/title.core.inc).
I only hit this error when changing the language of a node and I suspect that the error is unrelated to this patch. Should I set to RTBC and open a new issue?

Pisco’s picture

I can confirm that the error mentioned in #27 is not related to #24! Should I set to RTBC and open a new issue?

plach’s picture

Status: Needs review » Postponed

This wlll be committed when the core issue finally gets in.

Unfortunately the error in #27 is present also in the new release, altough it seems to be harmless. Just posted a follow-up in the other issue.

plach’s picture

Status: Postponed » Needs review
Issue tags: +Needs tests
FileSize
2.33 KB

Still struggling against this one and the ET ml_edit_branch: with #24 when saving an existing translation the original field is overridden. The attached patch seems to be ok. This really needs some test, but I have no time to write them right now.

Status: Needs review » Needs work

The last submitted patch, title_et-1519930-30.patch, failed testing.

plach’s picture

Status: Needs work » Needs review

#1620986: Perform reverse synchronization more reliably is what actually I was trying to solve in #24 (in a wrong way).

plach’s picture

#30: title_et-1519930-30.patch queued for re-testing.

plach’s picture

Status: Needs review » Reviewed & tested by the community

Nice!

plach’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed.

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

  • Commit 728bca6 on 7.x-1.x, workbench by plach:
    Issue #1519930 by plach, steinmb: Use the upcoming entity_language()...