#965300: Change LANGUAGE_NONE to LANGUAGE_NOT_SPECIFIED; add LANGUAGE_NOT_APPLICABLE and LANGUAGE_MULTIPLE adds these new special languages as constants. Currently LANGUAGE_NONE (to be renamed in that issue to LANGUAGE_NOT_SPECIFIED) is allowed to be assigned to some things, like nodes and path aliases, however, these languages are added on as extra items to the language list in form functions.

We should let users assign LANGUAGE_NOT_APPLICABLE and LANGUAGE_MULTIPLE as well to things like files, nodes, etc. Since the list of special languages grows in Drupal 8 from 1 (in Drupal 7) to 4 with these additions (and the already present LANGUAGE_SYSTEM), we either want to have a common language list extension wrapper or add these languages as regular pieces in the language table and then figure out #1314250: Allow filtering/configuration of which languages apply to what (UI, nodes, files, etc). Both have disadvantages. If we extend the list occasionally in code, we need to remember to do it at all times when applied to the same thing (such as for nodes in the filter UI and the node forms). If we decide to add them as languages in the language table, then we'll need to filter that down when needed, such as when UI language is considered, where these special languages have no meaning (except LANGUAGE_SYSTEM).

BTW solving #1314250: Allow filtering/configuration of which languages apply to what (UI, nodes, files, etc) would have other benefits too, eg. you could configure you don't want certain special languages to show up for nodes or files and always enforce an exact language selection, etc.

Currently both LANGUAGE_NONE and LANGUAGE_SYSTEM are added occasionally when needed to the language list.

Marking postponed on #965300: Change LANGUAGE_NONE to LANGUAGE_NOT_SPECIFIED; add LANGUAGE_NOT_APPLICABLE and LANGUAGE_MULTIPLE landing.

CommentFileSizeAuthor
#116 1471432-116.patch34.82 KBvasi1186
#114 1471432-114.patch34.83 KBvasi1186
#114 interdiff-112-114.txt7.53 KBvasi1186
#112 1471432-112.patch34.74 KBvasi1186
#112 interdiff_110_112.txt18.32 KBvasi1186
#110 1471432-109.patch36.21 KBvasi1186
#107 1471432-105.patch37.98 KBSchnitzel
#104 interdiff-100-104.txt4.59 KBSchnitzel
#104 1471432-104.patch37.93 KBSchnitzel
#102 1471432-100.patch38.77 KBSchnitzel
#101 1471432-99.patch155.29 KBSchnitzel
#97 1471432-97.patch38.68 KBvasi1186
#97 interdiff_92_97.txt8.34 KBvasi1186
#95 1471432-95.patch35.64 KBvasi1186
#92 1471432-92.patch33.41 KBvasi1186
#91 1471432-91.patch33.85 KBvasi1186
#87 1471432-87.patch33.77 KBvasi1186
#85 1471432-85.patch165.88 KBvasi1186
#77 1471432-77.patch32.81 KBGábor Hojtsy
#72 interdiff-1471432-70-72.txt1.63 KBkalman.hosszu
#72 1471432-72.patch33.7 KBkalman.hosszu
#70 1471432-70.patch32.07 KBkalman.hosszu
#70 67-70-interdiff.txt431 byteskalman.hosszu
#68 61-67-interdiff.txt8.43 KBkalman.hosszu
#67 1471432-67.patch32.1 KBkalman.hosszu
#61 1471432-61.patch34.61 KBkalman.hosszu
#55 locale-special-languages-ui-1471432-55.patch43.1 KBnod_
#53 locale-special-languages-ui-1471432-53.patch37.71 KBnod_
#48 locale-special-languages-ui-1471432-48.patch43.31 KBGábor Hojtsy
#46 locale-special-languages-ui-1471432-46.patch42.65 KBGábor Hojtsy
#44 locale-special-languages-ui-1471432-45.patch42.15 KBGábor Hojtsy
#41 locale-special-languages-ui-1471432-41.patch45.29 KBGábor Hojtsy
#38 20120403-qjex5hc59uhrgy1kemrbnei3d6.png114.54 KByoroy
#37 locale-special-languages-ui-1471432-35.patch30.18 KBGábor Hojtsy
#36 LanguageList.png93.03 KBGábor Hojtsy
#36 locale-special-languages-ui-1471432-35.patch29.79 KBGábor Hojtsy
#35 locale-special-languages-ui-1471432-35.patch29.79 KBGábor Hojtsy
#33 locale-special-languages-ui-1471432-33.patch29.61 KBGábor Hojtsy
#31 locale-special-languages-ui-1471432-31.patch27.18 KBGábor Hojtsy
#29 locale-special-languages-ui-1471432-29.patch27.18 KBGábor Hojtsy
#24 locale-special-languages-ui-1471432-24.patch19.67 KBtobiasb
#22 locale-special-languages-ui-1471432-22.patch17.72 KBtobiasb
#20 locale-special-languages-ui-1471432-20.patch15.66 KBdawehner
#18 locale-special-languages-ui-1471432-18.patch14.69 KBtobiasb
#16 locale-special-languages-ui-1471432-16.patch14.7 KBtobiasb
#13 locale-special-languages-ui-1471432-13.patch9.27 KBtobiasb
#11 locale-special-languages-ui-1471432-11.patch9.28 KBtobiasb
#9 language_1471432.patch9.28 KBtobiasb
#7 locale-special-languages-ui-1471432-7.patch11.13 KBpixelite
#2 D8Languages.png170.13 KBGábor Hojtsy
#2 D8ContentLanguage.png57.08 KBGábor Hojtsy
#2 D8ContentList.png82.82 KBGábor Hojtsy
#2 expose-special-languages.patch11.29 KBGábor Hojtsy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

Status: Active » Needs review
FileSize
11.29 KB
82.82 KB
57.08 KB
170.13 KB

Ok, here is a starter patch to get some thinking going :) It build off of the concept of keeping these special languages, and just including them for display as an extension of the normal language list. So it introduces a language_list_prepare() that can take a list of flags and would add/remove languages based on those flags. It has the languages associated with flags built-in. This is not nearly as flexible as #1314250: Allow filtering/configuration of which languages apply to what (UI, nodes, files, etc), though I've added a drupal_alter() for kicks (although the $options are very limited, so an alter would not be able to tell much which one to include/remove for a specific content type for example).

I've applied this to the language list. Since this list is used as a summary dashboard for languages, it made sense to include all languages and use the opportunity to educate people on what are they:

D8Languages.png

This does not let people order the special languages (there is no point?) but they can move the normal languages to inbetween special languages. When saved/reloaded, the normal languages are lined up again all above the special languages of course.

I've also redone the interface translation filtering page dropdown with this new code, but no UI change there.

Finally, added the new languages to the node assignment dropdown and to the language name function so they are properly displayed in summaries:

D8ContentLanguage.png

D8ContentList.png

Note that making this work is not as simple as adding the special languages as options for the node dropdown though. There are various conditions in fields, entities, etc. where the translatability, etc. is tied to the 'und' code. We need to update those places to not let translatability for the other special codes either (IMHO).

Gábor Hojtsy’s picture

Note we could alternatively also go the other direction and put in all these special languages in the language table and filter out ones not applicable as the situation dictates. We can use the 'locked' pattern that the node_type table uses for non-deletable/non-editable (code defined) content types. That would let people to disable "Not specified" let's say so it would not show up for node creation (i18n module includes such a feature for sites that never want to have nodes that have no specified language). That would also make it possible to reoder the special languages which would have an effect in eg. the list for node assignment. Not sure it is any useful to have the special langauges intermixed that way with the normal languages. Anyway, so we have some options here :)

Kristen Pol’s picture

My 2 cents: If these will be added to the language list, I like the idea of being able to reorder them in the list in case someone wants them first or last or something else entirely.

Kristen

dawehner’s picture

Regarding #3

Everytime you do things on runtime, things are harder to solve when querying things and you need this on the sql level.
For example if you put them in the table language fallbacks might be easier to be managed.

+      $title .= '<div class="description">' . $language->description . '</div>';

This is somehow not a that big issues at this point, but it should be better check_plained.

pixelite’s picture

I think it would be important to allow admins to disable these special languages, either site-wide or on a per content type basis.

I also think that exposing these special languages on the node edit page as part of the language selection widget is potentially confusing. When you create a node for the first time, you're making two choices: will the node will be translatable and if so, what the language of the content currently being added is. Combining these in a single select box will be confusing for some site admins.

pixelite’s picture

The patch in #2 no longer applies. I'm attaching an updated patch.

Kristen Pol’s picture

Here are my thoughts on the UI/functionality:

  1. Special languages
    • I am okay with the current naming conventions for the special "languages" ("Not specified", "Not applicable", "Multiple").
    • It feels odd to me to mix these so directly with the added languages, but I don't like the idea of splitting it out to another table. One possibility would be to put the special languages into a group and the special languages would be indented a little to give it more separation.
  2. Weighting/ordering
    • I think it might be annoying that the new languages added are always added to the bottom (below the special languages, assuming special languages is at the bottom). But, since that is the standard mechanism, it likely needs to stay like that.
    • If the special languages were put into a group (like mentioned above), then the whole group could be draggable to order before or after the added languages. And, the individual special languages could be ordered within the group.
    • If System (English) is the only item that will not be reorderable, it might be good to add spacing in front of it to align it with the other items.
  3. Disabled options
    • It feels inconsistent to me that some columns show disabled / not applicable items but the Operations column does not. Not a big deal though and this is probably the standard way it is implemented.
  4. Content type options
    • I like the idea that the special languages can be disabled per content type, but then won't that extend to all entities as well. What would that UI look like?
tobiasb’s picture

FileSize
9.28 KB

I hope this patch is what Gábor wanted, but in my understanding this should be the right patch ;-)

Status: Needs review » Needs work

The last submitted patch, language_1471432.patch, failed testing.

tobiasb’s picture

Status: Needs work » Needs review
FileSize
9.28 KB

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-11.patch, failed testing.

tobiasb’s picture

Status: Needs work » Needs review
FileSize
9.27 KB

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-13.patch, failed testing.

Kristen Pol’s picture

I am getting a failed "hunk":

[kristen:drupal8]$ more core/modules/node/node.pages.inc.rej 
--- core/modules/node/node.pages.inc
+++ core/modules/node/node.pages.inc
@@ -184,9 +184,8 @@
     $form['langcode'] = array(
       '#type' => 'select',
       '#title' => t('Language'),
-      '#default_value' => (isset($node->langcode) ? $node->langcode : ''),
+      '#default_value' => (isset($node->langcode) ? $node->langcode : LANGUAGE_NOT_SPECIFIED),
       '#options' => $language_options,
-      '#empty_value' => LANGUAGE_NOT_SPECIFIED,
     );
   }
   else {

tobiasb’s picture

Status: Needs work » Needs review
FileSize
14.7 KB

Ok, found the stupid mistake and fixed some tests.

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-16.patch, failed testing.

tobiasb’s picture

Status: Needs work » Needs review
FileSize
14.69 KB

why on hell copies my eclipse ide text with "mac line-endings" and not with \n :(

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-18.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
15.66 KB

The central problem is that if you save a language the static cache in locale_language_url_rewrite_url is not resetted.

So if you reset the static caches, it works at least for the test in common.test.

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-20.patch, failed testing.

tobiasb’s picture

Status: Needs work » Needs review
FileSize
17.72 KB

I found the stupid bug, yeaah ;-)

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-22.patch, failed testing.

tobiasb’s picture

Status: Needs work » Needs review
FileSize
19.67 KB

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-24.patch, failed testing.

Gábor Hojtsy’s picture

@tobiasb: good progress! Thanks for working on this! Some notes:

1. The upgrade function should not call the API function that is also used on fresh installs unfortunately. It needs to hard-wire the same list to be safe to use the same data regardless of any future updates to the language list.

2. Looks to me instead of doing custom filtering at all times based on language_list() return values, we should make language_list() capable of filtering those languages out that are locked (which applies to many cases). Can you look into implementing it that way.

I'll also ask some other people to take a look, but this looks like it is a better direction at the end that my original patch :)

Gábor Hojtsy’s picture

Title: Let users assign LANGUAGE_NOT_APPLICABLE and LANGUAGE_MULTIPLE » Let users configure and assign special languages

Make the title friendlier :)

klonos’s picture

Title: Let users configure and assign special languages » Let users configure and assign special languages (LANGUAGE_NOT_APPLICABLE & LANGUAGE_MULTIPLE)

...make the title easier to search for ;)

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
27.18 KB

Here is an update that makes language_list() filterable by two built-in filters. One is the only_enabled filter that was there before. Now it has a much more telling constant bit to tell the function to filter for that. Also added a locked filter. Used that in places instead to "declaratively" tell the system which languages we need at places instead of the procedural code to filter at all times.

Did not fix the update function. Will likely still have at least as many fails as before :)

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-29.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
27.18 KB

I should use proper bitwise operators :) LANGUAGE_FILTER_DISABLED | LANGUAGE_FILTER_LOCKED is the right bit combination instead of LANGUAGE_FILTER_DISABLED & LANGUAGE_FILTER_LOCKED which nulls them out and will not filter by either. Ha.

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-31.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
29.61 KB

Fix upgrade path by not using API functions in the update and putting in the special languages soon enough in upgrade bootstrap prepare. Passes upgrade tests for me locally.

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-33.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
29.79 KB

language_default() should also specify that it is unlocked. This should complete the patch hopefully :)

Gábor Hojtsy’s picture

Some more cleanups.

- Don't add LANGUAGE_SYSTEM and LANGUAGE_ALL as special languages, since these only ever apply to the interface translation realm and not elsewhere. These are already handled in their own way there from before the patch.
- Reinstantiated descriptions for the three more universal special languages, so the language list looks like this (when English UI translation is also enabled):

LanguageList.png

Let's get this reviewed!

Gábor Hojtsy’s picture

Did not roll that properly. Rerolled here.

yoroy’s picture

Gabor showed off this screen in IRC, of course I had some niggles about the ui text. Only minor ones this time :)

screenshot with suggestions for shorter ui copy

- I'm not a fan of the 'may be used' pattern, just use 'can' or 'use this' wherever possible
- We discussed my suggestion for 'No language specified' while I was typing this and decided that it wasn't an improvement, so lets leave that as it is :)

dawehner’s picture

Status: Needs review » Needs work
+++ b/core/includes/bootstrap.incundefined
@@ -2509,36 +2526,43 @@ function language_multilingual() {
+function language_list($filter = NULL) {

The docs of the function needs the change as well.

+++ b/core/includes/locale.incundefined
@@ -380,7 +380,7 @@ function locale_language_switcher_session($type, $path) {
+  $languages = language_list(LANGUAGE_FILTER_DISABLED);

I'm just wondering why here the filter for locked is not applied.

+++ b/core/modules/language/language.installundefined
@@ -9,8 +9,48 @@
+function language_special_languages() {

Some people could nitpick that this function doesn't have a @return.

+++ b/core/modules/language/language.installundefined
@@ -101,3 +148,49 @@ function language_update_8000() {
+    $max_weight = db_query('SELECT MAX(weight) FROM {language}')->fetchField();
+    $languages = array();
+    $languages[LANGUAGE_NOT_SPECIFIED] = array(
+      'langcode' => LANGUAGE_NOT_SPECIFIED,
+      'name' => 'Not specified',
+      'weight' => ++$max_weight,
+    );
+    $languages[LANGUAGE_NOT_APPLICABLE] = array(
+      'langcode' => LANGUAGE_NOT_APPLICABLE,
+      'name' => 'Not applicable',
+      'weight' => ++$max_weight,
+    );
+    $languages[LANGUAGE_MULTIPLE] = array(
+      'langcode' => LANGUAGE_MULTIPLE,
+      'name' => 'Multiple',
+      'weight' => ++$max_weight,

I'm wondering whether this could/should be simplified a bit by using language_special_languages() from above.

Gábor Hojtsy’s picture

@dawhener: I agree with everything but your last comment. Update functions should not reuse any functions that are not purely used in updates, since they might change and might have different results depending on when they ran, which makes the upgrade path unpredictable. Therefore update functions need things hardwired and/or need to use update specific versions of functions which never call outside hooks if at all possible.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
45.29 KB

Fixed both things mentioned by @dawehner and @yoroy. Also decided this would be a right time to make the jump to switch language_list() and make it filter by default for the "front-end languages". @catch suggested before that filtering for enabled languages on demand is not a good idea since we likely want to have that more often. Well, I did a deep check of language_list() calls for this patch, and the results are varied. Disabled and special languages are allowed for assignment on the backend but then disabled languages are not displayed to the user (and path aliases in disabled languages, etc. are not used). The question on WTH are disabled languages is "those can be used for content/translation/path alias, etc. assignment but are not displayed on the front end until the switch is flipped". It is like an unpublished node you are working on that is not showing up on the site. An unpublished language will not show in language switchers and user language selection, but will be assignable on the backend (such as for nodes).

So this flipped approach makes it much easier to figure out where are the disabled and locked languages used. Since this flips all language_list() use, it might fail again at some places :)

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-41.patch, failed testing.

Gábor Hojtsy’s picture

Assigned: Unassigned » Gábor Hojtsy

Working on rerolling this for all the different stuff moving around.

Gábor Hojtsy’s picture

Title: Let users configure and assign special languages (LANGUAGE_NOT_APPLICABLE & LANGUAGE_MULTIPLE) » Rework language_list(), let people use more special languages
Status: Needs work » Needs review
FileSize
42.15 KB

Retitled the issue for the current approach. I think its better if the special language constants are not there to scare eveybody away. Despite asking so many people to look into this, it was pretty well neglected, so let's try and make it more inviting :) I think its not a very complicated patch.

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-45.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
42.65 KB

Fixed a small code glitch not referencing language properly and a disabled language test not loading all disabled languages too. Should pass more tests.

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-46.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
43.31 KB

Two more tiny places where language objects were not referenced properly and resulted in failures.

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-48.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Postponed

After discussion with @catch, marking postponed on #1539072: Support for disabled languages broken, drop it. See IRC log there.

Gábor Hojtsy’s picture

Assigned: Gábor Hojtsy » Unassigned
Status: Postponed » Needs work

#1539072: Support for disabled languages broken, drop it landed so this needs a big reroll. Should simplify this a great deal. @kalmanhosszu was interested in working on this, so also unassigning.

nod_’s picture

Assigned: Unassigned » nod_
nod_’s picture

Status: Needs work » Needs review
FileSize
37.71 KB

There is a life outside JS :)

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-53.patch, failed testing.

nod_’s picture

Status: Needs work » Needs review
FileSize
43.1 KB

Ok not quite sure what I'm doing but we'll see.

Status: Needs review » Needs work

The last submitted patch, locale-special-languages-ui-1471432-55.patch, failed testing.

Gábor Hojtsy’s picture

Thanks! @nod_: disabled languages were removed in #1539072: Support for disabled languages broken, drop it, so LANGUAGE_ADD_DISABLED and all its use should not be there anymore. :) The previous test fails were around disabled languages, so this should also help with making the patch work :)

Gábor Hojtsy’s picture

@nod_: are you actually working on this? You are keeping a hold by being assigned.

nod_’s picture

Assigned: nod_ » Unassigned

ohh sorry about that, I thought Artusamak was on it. I'm not sure i'll have time to work on that soon enough.

kalman.hosszu’s picture

Assigned: Unassigned » kalman.hosszu

I can dedicate time for this issue.

kalman.hosszu’s picture

Status: Needs work » Needs review
FileSize
34.61 KB

Let's try an updatet patch.

Status: Needs review » Needs work

The last submitted patch, 1471432-61.patch, failed testing.

kalman.hosszu’s picture

Assigned: kalman.hosszu » Unassigned

I have to go off on the weekend, so I'll able to restart the work on this on monday.

Gábor Hojtsy’s picture

Are you still working on this?

kalman.hosszu’s picture

Assigned: Unassigned » kalman.hosszu

I can work on it.

Gábor Hojtsy’s picture

A quick review:

+++ b/core/includes/language.incundefined
@@ -103,22 +103,6 @@ function language_types_get_configurable($stored = TRUE) {
 /**
- * Disables the given language types.
- *
- * @param $types
- *   An array of language types.
- */
-function language_types_disable($types) {
-  $enabled_types = variable_get('language_types', language_types_get_default());
-
-  foreach ($types as $type) {
-    unset($enabled_types[$type]);
-  }
-
-  variable_set('language_types', $enabled_types);
-}
-

This should not be removed. Language types can still be disabled.

+++ b/core/includes/update.incundefined
@@ -188,38 +188,10 @@ function update_prepare_d8_language() {
-
-    // Rename the languages table to language.
-    db_rename_table('languages', 'language');
-
-    // Install/enable the language module. We need to use the update specific
-    // version of this function to ensure schema conflicts don't happen due to
-    // our updated data.
-    $modules = array('language');
-    update_module_add_to_system($modules);
-    update_module_enable($modules);
-
-    // Rename 'language' column to 'langcode'.
-    db_drop_primary_key('language');
-    $langcode_spec = array(
-      'type' => 'varchar',
-      'length' => 12,
-      'not null' => TRUE,
-      'default' => '',
-      'description' => "Language code, e.g. 'de' or 'en-US'.",
-    );
-    db_change_field('language', 'language', 'langcode', $langcode_spec, array('primary key' => array('langcode')));
-
-    // Update the 'language_default' system variable with the langcode change.
-    $language_default = variable_get('language_default');
-    if (!empty($language_default)) {
-      if (isset($language_default->language)) {
-        $language_default->langcode = $language_default->language;
-        unset($language_default->language);
-      }
-      unset($language_default->enabled);
-      variable_set('language_default', $language_default);
-    }
+    // Rename 'language' column to 'langcode', add locked column.
+    require_once DRUPAL_ROOT . '/core/modules/language/language.install';
+    language_update_8000();

Recent changes in the updates pointed to moving the code into the update bootstrap (here) instead of in misleading update function that are invoked directly. @catch requested we move stuff here instead of confusing update functions, so let's not remove this code.

+++ b/core/modules/language/language.installundefined
@@ -90,3 +139,77 @@ function language_schema() {
+
+/**
+ * Rename {language}.language to {language}.langcode.
+ *
+ * @see update_prepare_d8_language()
+ */
+function language_update_8000() {
+  // Rename language column to langcode and set it again as the primary key.
+  if (db_field_exists('language', 'language')) {
+    db_drop_primary_key('language');
+    $langcode_spec = array(
+      'type' => 'varchar',
+      'length' => 12,
+      'not null' => TRUE,
+      'default' => '',
+      'description' => "Language code, e.g. 'de' or 'en-US'.",
+    );
+    db_change_field('language', 'language', 'langcode', $langcode_spec, array('primary key' => array('langcode')));
+  }
+
+  // Update the 'language_default' system variable, if configured.
+  $language_default = variable_get('language_default');
+  if (!empty($language_default) && isset($language_default->language)) {
+    $language_default->langcode = $language_default->language;
+    unset($language_default->language);
+    variable_set('language_default', $language_default);
+  }
+}

As said above, do not move this out of the upgrade bootstrap.

+++ b/core/modules/language/language.installundefined
@@ -90,3 +139,77 @@ function language_schema() {
+/**
+ * Adds the locked column and saves the special languages.
+ */
+function language_update_8001() {
+  if (!db_field_exists('language', 'locked')) {
+    $locked_spec = array(
+      'type' => 'int',
+      'size' => 'tiny',
+      'not null' => TRUE,
+      'default' => 0,
+      'description' => 'A boolean indicating whether the administrator can edit or delete the language.',
+    );
+    db_add_field('language', 'locked', $locked_spec);
+
+    $max_weight = db_query('SELECT MAX(weight) FROM {language}')->fetchField();
+    $languages = array();
+    $languages[LANGUAGE_NOT_SPECIFIED] = array(
+      'langcode' => LANGUAGE_NOT_SPECIFIED,
+      'name' => 'Not specified',
+      'weight' => ++$max_weight,
+    );
+    $languages[LANGUAGE_NOT_APPLICABLE] = array(
+      'langcode' => LANGUAGE_NOT_APPLICABLE,
+      'name' => 'Not applicable',
+      'weight' => ++$max_weight,
+    );
+    $languages[LANGUAGE_MULTIPLE] = array(
+      'langcode' => LANGUAGE_MULTIPLE,
+      'name' => 'Multiple',
+      'weight' => ++$max_weight,
+    );
+    foreach ($languages as $language) {
+      db_insert('language')
+        ->fields(array(
+          'langcode' => $language['langcode'],
+          'name' => $language['name'],
+          'weight' => $language['weight'],
+          // These languages are locked, default to enabled.
+          'locked' => 1,
+          'enabled' => 1,
+        ))
+        ->execute();
+    }
+  }

Integrate this directly in the upgrade bootstrap.

+++ b/core/modules/language/language.moduleundefined
@@ -187,8 +196,8 @@ function language_save($language) {
+  // Update language count based on locked language count.
+  variable_set('language_count', db_query('SELECT COUNT(langcode) FROM {language} WHERE locked = 0')->fetchField());

Based on *un*locked count, right? :)

+++ b/core/modules/translation/translation.moduleundefined
@@ -138,6 +138,29 @@ function translation_form_node_form_alter(&$form, &$form_state) {
+    // Build two lists with the disabled and enabled languages.
+    $languages = language_list(LANGUAGE_ADD_LOCKED);
+    $grouped_languages = array();
+    foreach ($languages as $langcode => $language) {
+      $grouped_languages[(int) $language->enabled][$langcode] = $language;
+    }
+
+    $translator_widget = !empty($grouped_languages[0]) && user_access('translate content');
+    $groups = array(t('Disabled'), t('Enabled'));
+    // Allow translators to enter content in disabled languages. Translators
+    // might need to distinguish between enabled and disabled languages, hence
+    // we divide them in two option groups.
+    if ($translator_widget) {
+      $options = array();
+      foreach (array(1, 0) as $status) {
+        $group = $groups[$status];
+        foreach ($grouped_languages[$status] as $langcode => $language) {
+          $options[$group][$langcode] = $language->name;
+        }
+      }
+      $form['langcode']['#options'] = $options;
+    }

@@ -210,9 +233,9 @@ function translation_node_view(Node $node, $view_mode) {
+      // Do not show links to the same node, to unpublished translations or to
+      // translations in disabled languages.

Outdated code? We don't have disabled languages anymore. We should not have these groups here.

kalman.hosszu’s picture

Status: Needs work » Needs review
FileSize
32.1 KB

A quick update.

kalman.hosszu’s picture

FileSize
8.43 KB

An interdiff attached for the path above.

Gábor Hojtsy’s picture

Status: Needs review » Needs work

Nicer, smaller patch. Found this issue at a glance:

+++ b/core/includes/update.incundefined
@@ -189,37 +189,68 @@ function update_prepare_d8_language() {
+        db_insert('language')
+          ->fields(array(
+            'langcode' => $language['langcode'],
+            'name' => $language['name'],
+            'weight' => $language['weight'],
+            // These languages are locked, default to enabled.
+            'locked' => 1,
+            'enabled' => 1,
+          ))
+          ->execute();

Enabled is not a valid column anymore.

kalman.hosszu’s picture

Status: Needs work » Needs review
FileSize
431 bytes
32.07 KB

The "enabled" col is removed.

Status: Needs review » Needs work

The last submitted patch, 1471432-70.patch, failed testing.

kalman.hosszu’s picture

Status: Needs work » Needs review
FileSize
33.7 KB
1.63 KB

A little update in search tests, and an entity test fix.

Status: Needs review » Needs work
Issue tags: -D8MI, -sprint, -language-base

The last submitted patch, 1471432-72.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review

#72: 1471432-72.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +D8MI, +sprint, +language-base

The last submitted patch, 1471432-72.patch, failed testing.

kalman.hosszu’s picture

Assigned: kalman.hosszu » Unassigned

I have computer issues since last week so I can't work on it :(

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
32.81 KB

@kalman.hosszu: that is unfortunate to hear.

As for the patch, I don't agree with the search test changes, I think we discussed this over chat that if people don't have content in these possibly obscure languages, those languages should not show up in search. Hopefully there is an easy way to look up if we have search data in given languages, and display only those where we have.

Rerolled the patch since it had lots of other places where it did not apply 100% anymore. Removed the search test changes.

Status: Needs review » Needs work

The last submitted patch, 1471432-77.patch, failed testing.

Gábor Hojtsy’s picture

The tests failed with PDOException' with message 'SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupaltestbotmysql.simpletest995296language' doesn't exist and Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 53 bytes) in /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Statement.php on line 58 (which I don't think is related to this patch much).

Gábor Hojtsy’s picture

Status: Needs work » Needs review
Issue tags: -D8MI, -sprint, -language-base

#77: 1471432-77.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +D8MI, +sprint, +language-base

The last submitted patch, 1471432-77.patch, failed testing.

aries’s picture

Assigned: Unassigned » aries

Taken.

vasi1186’s picture

Status: Needs work » Needs review
Issue tags: -D8MI, -sprint, -language-base

#77: 1471432-77.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +D8MI, +sprint, +language-base

The last submitted patch, 1471432-77.patch, failed testing.

vasi1186’s picture

Status: Needs work » Needs review
FileSize
165.88 KB

The previous patch failed testing because: "Unable to apply patch."

Status: Needs review » Needs work

The last submitted patch, 1471432-85.patch, failed testing.

vasi1186’s picture

Status: Needs work » Needs review
FileSize
33.77 KB

previous patch wrong.

Status: Needs review » Needs work
Issue tags: -D8MI, -sprint, -language-base

The last submitted patch, 1471432-87.patch, failed testing.

vasi1186’s picture

Status: Needs work » Needs review

#87: 1471432-87.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +D8MI, +sprint, +language-base

The last submitted patch, 1471432-87.patch, failed testing.

vasi1186’s picture

Status: Needs work » Needs review
FileSize
33.85 KB

Fixed some issues in the update.inc.

vasi1186’s picture

FileSize
33.41 KB

diff between wrong commits in the previous patch...

Status: Needs review » Needs work

The last submitted patch, 1471432-92.patch, failed testing.

Gábor Hojtsy’s picture

So now we only have the memory issue left, it sounds like it goes in an infinite loop somewhere and eats up memory.

vasi1186’s picture

Status: Needs work » Needs review
FileSize
35.64 KB

Attached a new patch that should at least pass the tests :).

Status: Needs review » Needs work

The last submitted patch, 1471432-95.patch, failed testing.

vasi1186’s picture

FileSize
8.34 KB
38.68 KB

A new patch that should pass those 3 failed testcases.

vasi1186’s picture

Status: Needs work » Needs review
vasi1186’s picture

Issue tags: -D8MI, -sprint, -language-base

#97: 1471432-97.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +D8MI, +sprint, +language-base

The last submitted patch, 1471432-97.patch, failed testing.

Schnitzel’s picture

Status: Needs work » Needs review
FileSize
155.29 KB

Rerolling because of PSR-0 patches.

Schnitzel’s picture

FileSize
38.77 KB

oh, too much in this patch, my local git was a bit brocken.
fixxed now.

Gábor Hojtsy’s picture

Status: Needs review » Needs work

A few comments:

+++ b/core/includes/bootstrap.incundefined
@@ -245,6 +252,11 @@ const UNICODE_SINGLEBYTE = 0;
 /**
+ * Flag for language list extension to add locked languages.
+ */
+const LANGUAGE_ADD_LOCKED = 2;

Strange to have this as 2, when this is the only language_add constant.

+++ b/core/includes/update.incundefined
@@ -199,16 +199,70 @@ function update_prepare_d8_language() {
+    // Update the 'language_default' system variable, if configured.
+     $language_default = variable_get('language_default');
+    if (!empty($language_default) && isset($language_default->language)) {

Whitespace.

+++ b/core/modules/entity/lib/Drupal/entity/Entity.phpundefined
@@ -207,14 +207,15 @@ class Entity implements EntityInterface {
+      // If there is a langcode define for this field, just return it. Otherwise
+      // return the LANGUAGE_NOT_SPECIFIED.
+      return (isset($this->langcode)?$this->langcode:LANGUAGE_NOT_SPECIFIED);

'define' => 'defined'
'the LANGUAGE_' => 'LANGUAGE_'
whitespace issues with the operator (should have space before, after).

+++ b/core/modules/field/field.multilingual.incundefined
@@ -290,11 +290,25 @@ function field_language($entity_type, $entity, $field_name = NULL, $langcode = N
+        // If the field has a value for the one of the not applicable languages,
+        // then use that language for display. If not, the default one will be
+        // LANGUAGE_NOT_SPECIFIED.
+        $display_langcode[$instance['field_name']] = LANGUAGE_NOT_SPECIFIED;

'for the one of the' => 'for one of the'

+++ b/core/modules/language/lib/Drupal/language/Tests/LanguageUrlRewritingTest.phpundefined
@@ -48,7 +48,7 @@ class LanguageUrlRewritingTest extends WebTestBase {
-    $non_existing = language_default();
+    $non_existing = clone language_default();

This is the only place where this is being cloned. Why?!

+++ b/core/modules/search/lib/Drupal/search/Tests/SearchLanguageTest.phpundefined
@@ -29,8 +29,8 @@ class SearchLanguageTest extends SearchTestBase {
   function testLanguages() {
     // Check that there are initially no languages displayed.
-    $this->drupalGet('search/node');
-    $this->assertNoText(t('Languages'), t('No languages to choose from.'));
+    //$this->drupalGet('search/node');
+    //$this->assertNoText(t('Languages'), t('No languages to choose from.'));
 
     // Add predefined language.
     $edit = array('predefined_langcode' => 'fr');
@@ -62,7 +62,7 @@ class SearchLanguageTest extends SearchTestBase {

@@ -62,7 +62,7 @@ class SearchLanguageTest extends SearchTestBase {
     $this->drupalPost('admin/config/regional/language/delete/en', array(), t('Delete'));
 
     // Check that there are again no languages displayed.
-    $this->drupalGet('search/node');
-    $this->assertNoText(t('Languages'), t('No languages to choose from.'));
+    //$this->drupalGet('search/node');
+    //$this->assertNoText(t('Languages'), t('No languages to choose from.'));

Since these don't apply anymore, remove them? We don't leave commented out code in Drupal like that.

Schnitzel’s picture

Status: Needs work » Needs review
FileSize
37.93 KB
4.59 KB

new version of the patch, @vasi1186 did also some work here.

This is the only place where this is being cloned. Why?!

This is not needed anymore, because language_default() returns a new instance of the Object.

vasi1186’s picture

Issue tags: -D8MI, -sprint, -language-base

#104: 1471432-104.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +D8MI, +sprint, +language-base

The last submitted patch, 1471432-104.patch, failed testing.

Schnitzel’s picture

Status: Needs work » Needs review
FileSize
37.98 KB

reroll because of PSR-0 testlove.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Looks good!

webchick’s picture

Status: Reviewed & tested by the community » Needs work

Nice work! Overall this looks fine, but coming into it fresh I had a couple of observations/comments:

 /**
+ * The language code used when referring to all languages.
+ *
+ * @todo W3C uses this for 'Allar', need to find a better code.
+ */
+const LANGUAGE_ALL = 'all';

…

 /**
+ * Flag for language list extension to add locked languages.
+ */
+const LANGUAGE_ADD_LOCKED = 1;

Since we already have a constant that means "All of the languages," could we scrap this second one that's specific only to the language_list() function?

+      // If there is a langcode defined for this field, just return it. Otherwise
+      // return LANGUAGE_NOT_SPECIFIED.
+      return (isset($this->langcode)?$this->langcode:LANGUAGE_NOT_SPECIFIED);

(totally minor: there should be spaces around ? and : here, per coding standards)

foreach (array(LANGUAGE_NOT_SPECIFIED, LANGUAGE_NOT_APPLICABLE, LANGUAGE_MULTIPLE) as $langcode_not_applicable) {

…

+      if (in_array($langcode, array(LANGUAGE_NOT_SPECIFIED, LANGUAGE_NOT_APPLICABLE, LANGUAGE_MULTIPLE))) {

…
etc.

There's a few places where we're manually specifying an array of these "special" constants. Since we went from 1 -> 3 in D8, and could conceivably (maybe) go from 3 -> 4 later in D8 or D9, should we add a simple helper function instead so if we later expand/contract this list we can do it in one place?

-  return array_keys(language_list() + array(LANGUAGE_NOT_SPECIFIED => NULL));
+  return array_keys(language_list(LANGUAGE_ADD_LOCKED) + array(LANGUAGE_MULTIPLE => NULL, LANGUAGE_NOT_SPECIFIED => NULL, LANGUAGE_NOT_APPLICABLE => NULL));

This in particular was confusing, and I asked Gábor about it, because I *thought* that the point of the LANGUAGE_ADD_LOCKED constant was to get me the list of languages, including the locked ones. He pointed out that language_list() does different logic depending on if the language system is loaded or not, and the function to figure out the special languages is part of Language module which may not be enabled on a given site.

So I suggested...

+function language_special_languages() {

Ah-ha. Here's the helper function I was looking for above. :)

So that this is available everywhere, it *probably* makes sense to move it to bootstrap.inc alongside language_list() (I can hear chx beating me in the head with a club suggesting adding more code to bootstrap.inc, but it honestly doesn't make sense to keep these decoupled.)

vasi1186’s picture

Status: Needs work » Needs review
FileSize
36.21 KB

Attached a new patch that should solve the issues from #109. The main changes:
- get rid of the LANGUAGE_ADD_LOCKED and use LANGUAGE_ALL instead.
- move the language_special_languages() function from language.install in bootstrap.inc and replace the duplicated code from update.inc and language.install to use this function.

Gábor Hojtsy’s picture

Status: Needs review » Needs work
+++ b/core/includes/bootstrap.incundefined
@@ -204,6 +204,13 @@ const LANGUAGE_NOT_APPLICABLE = 'zxx';
+ *
+ * @todo W3C uses this for 'Allar', need to find a better code.

This is not used in this version of the patch in place of real language codes anymore (due to the new UI patch landed), so this todo is outdated, it does not matter.

+++ b/core/includes/bootstrap.incundefined
@@ -2653,18 +2660,28 @@ function language_multilingual() {
-function language_list() {
+function language_list($flags = NULL) {

Now that it is just one flag, and we are not foreseeing more (since disabled languages are gone), I think we want to name this $all with a default of FALSE or something like that, and have LANGUAGE_ALL possibly defined as TRUE.

+++ b/core/includes/bootstrap.incundefined
@@ -2677,8 +2694,56 @@ function language_list() {
+  // Filter the full list of languages based on the criteria in $flags. By
+  // default we remove the disabled and locked languages, but the caller may
+  // request for those languages to be added as well.

No disabled languages anymore.

+++ b/core/includes/bootstrap.incundefined
@@ -2677,8 +2694,56 @@ function language_list() {
+/**
+ * Returns a list with the special languages.
+ *
+ * @return
+ *   An array of language objects.
+ */
+function language_special_languages() {
+  $locked_language = array(
+    'default' => FALSE,
+    'locked' => TRUE,
+    'enabled' => TRUE,
+  );
+
+  // The special languages should always have a big weight.
+  $weight = 20;
+  $languages = array();

Well, to make sure this will always work, let's have a $weight argument on this function and count from there. Pass in the existing max weight from the outside, since it can be different based on the circumstances (so it works best all the time). Sites can have 20 languages (localize.drupal.org has over a 100 :). I'd hate to need to hack core to make it work :)

+++ b/core/includes/bootstrap.incundefined
@@ -2720,6 +2785,16 @@ function language_name($langcode) {
 /**
+ * Checks if a language is not applicable.
+ *
+ * @param $langcode
+ *   The language code.
+ */
+function language_not_applicable($langcode) {
+  return array_key_exists($langcode, language_special_languages());
+}

Naming for this function seems misleading, since we have exactly one specific language that is named not applicable. So maybe name this language_is_locked or something :)

+++ b/core/includes/update.incundefined
@@ -199,16 +199,54 @@ function update_prepare_d8_language() {
+            // These languages are locked, default to enabled.

"default to enabled" is not a relevant comment here, that already happens elsewhere.

+++ b/core/modules/field/field.multilingual.incundefined
@@ -165,14 +165,15 @@ function _field_language_suggestion($available_langcodes, $langcode_suggestion,
- * The language codes that may be associated to fields include
- * LANGUAGE_NOT_SPECIFIED.
+ * The languages that may be associated to fields may include also the not
+ * applicable languges like LANGUAGE_NOT_SPECIFIED, LANGUAGE_NOT_APPLICABLE or
+ * LANGUAGE_MULTIPLE.
...
+  return array_keys(language_list(LANGUAGE_ALL));

I don't think we need to include any of these comments here anymore, the code under it is very descriptive with language_list(LANGUAGE_ALL) in itself.

+++ b/core/modules/field/field.multilingual.incundefined
@@ -290,11 +291,25 @@ function field_language($entity_type, $entity, $field_name = NULL, $langcode = N
+        // If the field has a value for one of the not applicable languages,
+        // then use that language for display. If not, the default one will be
+        // LANGUAGE_NOT_SPECIFIED.
+        $display_langcode[$instance['field_name']] = LANGUAGE_NOT_SPECIFIED;
+        foreach (language_special_languages() as $language_not_applicable) {
+          if (isset($entity->{$instance['field_name']}[$language_not_applicable->langcode])) {
+            $display_langcode[$instance['field_name']] = $language_not_applicable->langcode;
+            break;

+++ b/core/modules/field/lib/Drupal/field/Tests/TranslationTest.phpundefined
@@ -263,6 +263,9 @@ class TranslationTest extends FieldTestBase {
+    // This array is used to store, for each field name, which one of the not
+    // applicable languages will be used for display.
+    $not_applicable_languages = array();

@@ -277,6 +280,15 @@ class TranslationTest extends FieldTestBase {
+      // If the langcode is one of the not applicable languages, then that one
+      // will also be used for display. Otherwise, the default one should be
+      // used, which is LANGUAGE_NOT_SPECIFIED.
+      if (language_not_applicable($langcode)) {
+        $not_applicable_languages[$field_name] = $langcode;
+      }
+      else {
+        $not_applicable_languages[$field_name] = LANGUAGE_NOT_SPECIFIED;
+      }

@@ -286,14 +298,13 @@ class TranslationTest extends FieldTestBase {
+      $this->assertTrue($display_langcodes[$field_name] == $not_applicable_languages[$field_name], t('The display language for field %field_name is %language.', array('%field_name' => $field_name, '%language' => $not_applicable_languages[$field_name])));

Strange terminology :) ..._special_languages() as $...not_applicable. They are really the locked languages. There is only *one* not applicable language so to speak.

+++ b/core/modules/language/lib/Drupal/language/Tests/LanguageListTest.phpundefined
@@ -99,6 +99,8 @@ class LanguageListTest extends WebTestBase {
+    $langs = language_list(LANGUAGE_ALL);
+    debug($langs);

debug

+++ b/core/modules/translation/translation.moduleundefined
@@ -210,9 +210,9 @@ function translation_node_view(Node $node, $view_mode) {
+      // Do not show links to the same node, to unpublished translations or to
+      // translations in disabled languages.

There are no disabled languages anymore.

vasi1186’s picture

Status: Needs work » Needs review
FileSize
18.32 KB
34.74 KB

Attached the new patch and an interrdiff between 110 and 112.

Gábor Hojtsy’s picture

Status: Needs review » Needs work
+++ b/core/includes/bootstrap.incundefined
@@ -206,7 +206,6 @@ const LANGUAGE_MULTIPLE = 'mul';
- * @todo W3C uses this for 'Allar', need to find a better code.
  */
 const LANGUAGE_ALL = 'all';

We said this would be TRUE :) It is not used anywhere else in core after your previous patch, right? :) So it would not make sense if we apply this patch, right? See below too.

+++ b/core/includes/bootstrap.incundefined
@@ -2660,15 +2659,15 @@ function language_multilingual() {
+ * @param $all
+ *   (optional) A flag depending on the need for locked
+ *   languages in the returned list.

Go until 80 chars, don't need to wrap so early.

+++ b/core/includes/bootstrap.incundefined
@@ -2757,7 +2757,7 @@ function language_special_languages() {
-  $languages = language_list(LANGUAGE_ALL);
+  $languages = language_list(TRUE);

No, I think we agreed that we keep LANGUAGE_ALL as TRUE, so the invocation pattern is clear, and people are not wondering WTH is TRUE there. Same everywhere.

vasi1186’s picture

Status: Needs work » Needs review
FileSize
7.53 KB
34.83 KB

OK, let's see this one then.

Gábor Hojtsy’s picture

Status: Needs review » Needs work
--- a/core/includes/bootstrap.inc
+++ b/core/includes/bootstrap.incundefined

+++ b/core/includes/bootstrap.incundefined
@@ -207,7 +207,7 @@ const LANGUAGE_MULTIPLE = 'mul';

@@ -207,7 +207,7 @@ const LANGUAGE_MULTIPLE = 'mul';
  * The language code used when referring to all languages.
  *
  */
-const LANGUAGE_ALL = 'all';

Just one note now: one empty line too much at the end of code docs.

vasi1186’s picture

Status: Needs work » Needs review
FileSize
34.82 KB

Removed the line.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Should be RTBC based on all reviews and the previous one being green then.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome! Even better than what I asked for. :)

Committed and pushed to 8.x. Thanks!

vasi1186’s picture

Status: Fixed » Needs review
Issue tags: -D8MI, -sprint, -language-base

#116: 1471432-116.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +D8MI, +sprint, +language-base

The last submitted patch, 1471432-116.patch, failed testing.

vasi1186’s picture

Status: Needs work » Fixed
Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

Added a changelog entry at http://drupal.org/node/1647438.

Gábor Hojtsy’s picture

Issue tags: -sprint

An important followup: #1649400: Make locked/special languages fully extensible.

Moving this off the sprint board as it is done as-is.

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

Anonymous’s picture

Issue summary: View changes

Added more notes on why http://drupal.org/node/1314250 would be useful.