Add langcode property to all entity types; for the user entity, distinguish entity language from user's language preference

First of all, thanks for this fantastic patch! I spent considerable time with reviewing this patch today (and we discussed this in detail while you wrote it last Friday). All-in-all I think it looks very solid. The initialization for files and terms/vocabularies for upgraded sites looks good. The snippets to sync the user preferred/entity language by default look great. We can retain our simple UI while providing flexibility for those who want to separate preferred language from entity language. I only found very minor issues that I think @xjm would point out sooner or later :)

+++ b/core/modules/locale/locale.moduleundefined
@@ -236,17 +236,41 @@ function locale_language_selector_form($user) {
+ * #value_callback of user register and profile forms' langcode element.

+++ b/core/modules/system/system.installundefined
@@ -1666,6 +1673,40 @@ function system_update_8002() {
+ * Add the {file_managed}.langcode field.

+++ b/core/modules/taxonomy/taxonomy.installundefined
@@ -245,4 +259,44 @@ function taxonomy_field_schema($field) {
+ * Add a langcode field to {taxonomy_term_data} and {taxonomy_vocabulary}.

+++ b/core/modules/user/user.installundefined
@@ -354,5 +361,40 @@ function user_update_8000() {
+ * Split the {users}.language field to langcode and preferred_langcode.

Summaries should start with 3rd person verbs now.

PS. also asked @fubhy and @tstoeckler to review this patch, hope they can get to it soon.

Leaving needs review since the issues are very minor.

3rd person verbs are fixed.

Regarding the update hook docs:
#1414640: No documentation standard for hook_update_N() summaries [policy, no patch]

@xjm: In other words, its fine as it is now :)

Maybe this is just nitpicking, but those two issues are the very last things that I noticed. Feel free to ignore them as they probably are irrelevant.

+++ b/core/modules/locale/locale.moduleundefined
@@ -236,17 +236,41 @@ function locale_language_selector_form($user) {
+    // than the preferred_langcode element. Set a large weight here in case
+    // module's alter the weight of the other element.

This should just read "modules" (not module's). Maybe even rephrase it to "... in case a module alters the weight of the other element."

+++ b/core/modules/locale/locale.moduleundefined
@@ -236,17 +236,41 @@ function locale_language_selector_form($user) {
 /**
+ * Sets value of user register and profile forms' langcode element.

This is a private function so it doesn't really matter but maybe we could use proper grammar like "Sets the value of the user register and profile forms' langcode element."

Other than that it looks nice and ready to go.

Those two text change suggestions are great IMHO, so here they are. Any other concerns?

This looks good to me. I'm hoping that we can standardize some of this code soon into entity.module.

I think a lot of eyes have +1 this in addition to me so RTBC.

I've tested and it works perfectly.

This is indeed a great first step towards having language support enabled on any entity (the next logical step would be to factor out the language support logic/ui and move them to the base entity object/module). Just a couple of notes:

+++ b/core/includes/file.inc
@@ -568,6 +568,9 @@ function file_load($fid) {
+  if (!isset($file->langcode)) {
+    $file->langcode = LANGUAGE_NONE;
+  }

+++ b/core/modules/taxonomy/taxonomy.admin.inc
@@ -117,6 +117,8 @@ function taxonomy_form_vocabulary($form, &$form_state, $edit = array()) {
+      'langcode' => language_default()->langcode,

@@ -659,6 +661,8 @@ function taxonomy_form_term($form, &$form_state, $edit = array(), $vocabulary =
+      'langcode' => language_default()->langcode,

The related update functions nicely explain the reasons why they are defaulting to the (different) values above. Also these lines could use an explicit comment clarifying them.

+++ b/core/modules/system/system.install
@@ -1666,6 +1673,40 @@ function system_update_8002() {
+    // According to the documentation of db_change_field(), indeces using the
+    // field should be dropped first; if the contrib module created any indeces,
+    // it is its responsibility to drop them in an update function that runs
+    // before this one, which it can enforce via hook_update_dependencies().

+++ b/core/modules/taxonomy/taxonomy.install
@@ -245,4 +259,44 @@ function taxonomy_field_schema($field) {
+      // According to the documentation of db_change_field(), indeces using the
+      // field should be dropped first; if the contrib module created any
+      // indeces, it is its responsibility to drop them in an update function
+      // that runs before this one, which it can enforce via
+      // hook_update_dependencies().

This looks sensible to me, I'm just wondering what is supposed to happen in the case of the Entity translation module, which is the most likely candidate to mess with the entity 'langcode' columns in D7 (see #1280546: Introduce a language selection widget for every entity). Since it is going to be moved to core in D8, who will be responsible to perform this update properly? I'd say needing to install an 8.x version of the ET module just to be able to upgrade would be nonsensical.

26 days to next Drupal core point release.

@plach: re your first note, this is being addressed with #258785: Provide more flexible settings for initial language on content types and the code in question added by this patch is likely not there for longer than a month or two. Let's not ping-pong this with that given we want to move setting of defaults to a versatile UI.

Your second note did not have any consequences for the status of this patch. I think that question comes up when/if ET introduces a language property on entities. We can *then* add the same update code like we have for users except with a condition for whether a language field existed if we want to solve that in core. We usually don't have migration path for contrib modules in core, so the other alternative is for ET to have a D7 release which people should update to before updating to D8. That release would prepare the entity tables to have langcode like D8 expects, in which case we already have code in this patch to cover that situation.

Let's have this land and work towards flexible defaults in #258785: Provide more flexible settings for initial language on content types.

Here is the requested doc update anyway, since the patch needed a reroll due to another D8MI patch landing. It is just a doc change as per requested, should not change RTBC status.

Note once again that parts of this patch were worked on by MANY people and it was reviewed by *many* people earlier in previous issues at #1444966: Add langcode property in taxonomy schema, #1439680: Rename $user language property to langcode, #1444992: Add langcode property in file schema, so if this issue looks slim and unrealistic to work out a change of this scale, just add up the existing work from all of those issues.

Committed to 8.x. Great start for adding better language support to entities.

There was a collision with update numbers, but I just went in and fixed it. http://drupalcode.org/project/drupal.git/blobdiff/2d6dcdd509126c6ab86e0c...

Fantastic! Thanks, removing sprint tag.

Now onto making language based data accessibility (fields/properties) much simpler at #1277776: Add generic field/property getters/setters (with optional language support) for entities.