The TF UI got moved to D8 / contrib, however there's still a prominent translatable or not option when creating a field via the field UI. It seems to me like this should move into the contrib module now.

Files: 
CommentFileSizeAuthor
#4 field_ui-654934-4.patch1.38 KBplach
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_ui-654934-4.patch.
[ View ]

Comments

plach’s picture

This would make sense on the UX side, but we must consider that one might start creating content before actually installing Entity Translation. In this case one would have no way to change a field's translatability as field properties cannot be changed after content has been created.

By letting that choice in core we allow people plan the translatability of a field. We must also consider that Entity Translation might not be the only module exploiting translatable fields.

yched’s picture

Hm, I think you can change that property after the event now.

plach’s picture

In this case no objection on this.

plach’s picture

Status:Active» Needs review
StatusFileSize
new1.38 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_ui-654934-4.patch.
[ View ]

Removed field translatability selector.

yched’s picture

Status:Needs review» Reviewed & tested by the community
yched’s picture

Status:Reviewed & tested by the community» Needs review

Oops. This raises a couple additional questions :

- field_ui_field_overview_form_submit() creates field with 'translatable' => TRUE by default.
Do we still want this ?

- the (node) body field is created with 'translatable' => TRUE - node_configure_fields().
Do we still want this ?

plach’s picture

We already discussed this in #563998: Field UI: no way to set the translatable property: IMO the answer is still YES, because translatability is not taken into account if no translation handler is actually enabled. If one enables a translation handler, he probably wishes to translate fields and making all fields translatable replicates our core content translation pattern: a field for each language.

yched’s picture

Status:Needs review» Reviewed & tested by the community

"making all fields translatable replicates our core content translation pattern: a field for each language"

I don't get this last part, but the rest makes sense. Back to RTBC then.

webchick’s picture

Status:Reviewed & tested by the community» Needs review

I definitely like removing this from the UI, since core doesn't do anything with it, and it forces a confusing decision immediately into the face of anyone adding any field, regardless if they use i18n features or not, which most sites don't. So, committed to HEAD.

But I'm wondering if we could discuss a bit more (either here or in another issue) about the default value of this should be. Plach and I had a long talk in IRC earlier about this. The jist was that this flag is toggling the *translatability* of fields, not whether they have translations. And generally speaking, you will want most fields translatable, so the current default probably makes sense.

On the other hand, though, this won't actually take effect unless you use Entity Translation (or equivalent) in contrib. And it'd be possible there to toggle the fields to translatable during hook_install(), then put in a much more streamlined UI (for example a matrix of all of your fields with "select all" and stuff like) for controlling of toggling this property itself. Clicking "configure" on 50+ fields to check them doesn't strike me as my fun idea of a good time. :P

plach’s picture

"making all fields translatable replicates our core content translation pattern: a field for each language"
I don't get this last part, but the rest makes sense.

I meant that in core the only way to translate content is through the Translation module which creates a node for each translation, so fields are replicated for each translation. Exactly what happens if you don't use non-translatable fields.

I need to think a bit more about webchick's proposal: can we perform this change to default values after the alpha deadline?

plach’s picture

While working on #657972: Make field fallback logic actually reusable I changed my mind about this. I think that defaulting to FALSE should give a perfomance improvement on unilingual sites. See http://api.drupal.org/api/function/field_is_translatable/7, if a field is untranslatable we avoid a call to http://api.drupal.org/api/function/field_has_translation_handler/7 (for each field, for each field_attach_X() call).

catch?

catch’s picture

We have a static cache in field_is_translatable(), so even when inserting 1000 nodes in a migration, field_is_translatable() only gets called 26 times. It'd be quite a small optimization but shouldn't matter too much either way.

The main issue I'm seeing in migrations (which is what I'm mainly profiling at the moment), is that functions like field_is_translatable() are called per entity, per field, per field_invoke op - on this site that's 89,000 times to insert 1,000 nodes (29 fields * ($default + $insert + $presave))- ish. That's an issue with _field_invoke() overall (which can take up to 8% of a migration run), not with the translatable functionality as such though.

plach’s picture

@catch:

We have a static cache in field_is_translatable()

Do you mean field_available_languages()?

Edit:

That's an issue with _field_invoke() overall (which can take up to 8% of a migration run), not with the translatable functionality as such though.

Well, if this is not an issue strictly tied to translation, wouldn't this be almost the same scenario as in CCK? I mean, _field_invoke() already existed.

plach’s picture

A side thought: what about displaying the "translatable" checkbox only if Locale (or any other module exploiting Field's multilingual capability) is enabled? We have an API function for this: http://api.drupal.org/api/function/field_has_translation_handler/7.

yched’s picture

re catch #12 : I had #760238: Optimize _field_invoke() sitting on my hard disk. Does not touch repeated calls to field_is_translatable(), though.

retester2010’s picture

Issue tags:-translatable fields

#4: field_ui-654934-4.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+translatable fields

The last submitted patch, field_ui-654934-4.patch, failed testing.

Jax’s picture

Status:Needs work» Fixed
plach’s picture

Status:Fixed» Needs review

The issue has been set in 'needs review' status by webchick while committing.

Original issue: #563998: Field UI: no way to set the translatable property.

plach’s picture

Status:Needs review» Fixed

Talked with webchick: it's too late for changing our minds on this, hence marking fixed.

Status:Fixed» Closed (fixed)
Issue tags:-translatable fields

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