On 7.x it is there, but has the default of visible (incovenient, needs to be hidden for any sites I do).
Disappeared in 8.x.
Now I dont see the need for it in the display tab, but perhaps there is a use case where one might want to display the language on a node. SO lets re-add it

Gabor noted in #1074672:
indeed it not appearing on the manage display screen at all seems like (a separate) regression, that would need its own tests too. It only applies to Drupal 8. However, it sounds like an improvement that nodes would not have the language field displayed automatically anymore.

So, opening a new issue, I hope to work on it later this week :-)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Title: the language field is not visible on the manage display tab for fields » Regression: language field is not visible on manage display
Issue tags: +D8MI, +sprint, +language-content

Tagging up, cleaning title.

boran’s picture

by adding the following in node.module/node_field_extra_fields() after $extra['node'][$bundle->type]['form']['language'], the language will appear in the display tab.

$extra['node'][$bundle->type]['display']['language'] = array(
        'label' => t('Language'),
        'description' => $description,
        'weight' => 0,
      );

But it defaults to visible, where as it needs to be hidden.
Looked at core/modules/field/field.api.php and core/modules/field_ui/field_ui.api.php, but its not obvious to me what hook needs to be called set set the default visible of the language in "manage display".

Any suggestions?

Gábor Hojtsy’s picture

@boran: thanks for the info! Is this the same code that was present in Drupal 7? Did you manage to dig up any reasons as to why it was removed in Drupal 8?

Gábor Hojtsy’s picture

Issue tags: +Needs tests

Will also need tests. If there would have been tests for this, we would not have lost this.

boran’s picture

Status: Active » Needs review
FileSize
1.8 KB
601 bytes

Code and test attached.
Still one problem: how to set the default value of Language on the display tab to "hidden"?

D7: the i18n module handles this in D7.

Status: Needs review » Needs work

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

boran’s picture

Status: Needs work » Needs review

"The testbot client is probably malfunctioning." :-)

Gábor Hojtsy’s picture

#5: node_1757504_5_code.patch queued for re-testing.

Gábor Hojtsy’s picture

#5: node_1757504_5_test.patch queued for re-testing.

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

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

boran’s picture

I'm very new to tests, any suggestions on why the test fails? Why does for example FileFieldWidgetTest complain?

Also looking suggestions for

how to set the default value of Language on the display tab to "hidden"?

with a defautl of hidden the patch should not be committed, as it disturb the UI experience (will have to be manually set to hidden on most sites).

maxtorete’s picture

Assigned: boran » Unassigned
Status: Needs work » Needs review
FileSize
3.42 KB

I have made a patch to fix this issue. To achieve it, I have made these changes:

  • I have modified _field_info_prepare_extra_fields to accept 'visible' as an option for 'display' context.
  • I have update hook_field_extra_fields comment with this change.
  • I have modified node_field_extra_fields to set language visibility default value to hidden.
  • I have modified testNodeTypeInitialLanguageDefaults to tests if the language field exists on manage display tabs and it defaults to hidden.
maxtorete’s picture

FileSize
3.42 KB

This is the same patch as #12, but I trimmed some white spaces on blank line.

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

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

maxtorete’s picture

Status: Needs work » Needs review

#13: node_1757504_13.patch queued for re-testing.

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

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

maxtorete’s picture

Status: Needs work » Needs review
FileSize
4 KB

As said on #7, testbot is making some strange things... I attach a new patch (comment too long splitted on two lines).

maxtorete’s picture

FileSize
3.98 KB

I forgot to delete a useless line. Added patch with that correction.

c31ck’s picture

Status: Needs review » Needs work

This looks really good. The patch applies cleanly, the language field is shown on the manage display tab and it is hidden by default. However, when configuring the language to display and actually viewing the node, the language is not displayed. Do we need to make changes to node_build_content() to make this work?

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeTypeInitialLanguageTest.phpundefined
@@ -78,6 +78,13 @@ class NodeTypeInitialLanguageTest extends NodeTestBase {
+    $this->drupalGet('admin/structure/types/manage/article/display');
+    $language_display = $this->xpath('//*[@id="language"]');
+    $this->assert(!empty($language_field), 'Language field is visible on manage display tab.');

Shouldn't this be $this->assert(!empty($language_display) instead of $this->assert(!empty($language_field)?

+++ b/core/modules/node/node.moduleundefined
@@ -678,14 +678,20 @@ function node_field_extra_fields() {
+    // Visibility of the ordering of the language selector is the same as on ¶

Whitespace issue.

+++ b/core/modules/node/node.moduleundefined
@@ -678,14 +678,20 @@ function node_field_extra_fields() {
     if ($module_language_enabled && !variable_get('node_type_language_hidden_' . $bundle->type, TRUE)) {
       $extra['node'][$bundle->type]['form']['language'] = array(
         'label' => t('Language'),
         'description' => $description,
         'weight' => 0,
       );
+      $extra['node'][$bundle->type]['display']['language'] = array(
+        'label' => t('Language'),
+        'description' => $description,
+        'weight' => 0,
+        'visible' => FALSE,
+      );

I'm not sure if the 'Hide language selector' option should also control the display of the language field when viewing a node. I think it should only have an effect on the 'manage fields' tab and not on the 'manage display' tab.

maxtorete’s picture

Status: Needs work » Needs review
FileSize
4.17 KB

I'm not sure if the 'Hide language selector' option should also control the display of the language field when viewing a node. I think it should only have an effect on the 'manage fields' tab and not on the 'manage display' tab.

I think you're right, I have fixed this and the other issues you noticed. Thanks!

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

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

maxtorete’s picture

Status: Needs work » Needs review

#20: node_1757504_20.patch queued for re-testing.

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

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

maxtorete’s picture

Status: Needs work » Needs review
FileSize
4.17 KB

This could be a random issue on testbot (reported issue #1779638: Unexplained test failure in LocaleFileImportStatus->testBulkImportUpdateExisting()), locally it passes all tests. I'm sending it again to test.

maxtorete’s picture

However, when configuring the language to display and actually viewing the node, the language is not displayed. Do we need to make changes to node_build_content() to make this work?

I'm not sure if we should add it on translation_node_view on translation module or add it on node_view function on node module.

maxtorete’s picture

FileSize
4.77 KB

As the extra field for language info is added on node module, I have implemented it on node_view.

maxtorete’s picture

FileSize
4.77 KB

Comment without ending dot fixed.

Gábor Hojtsy’s picture

Status: Needs review » Needs work
+++ b/core/modules/node/lib/Drupal/node/Tests/NodeTypeInitialLanguageTest.phpundefined
@@ -78,6 +78,13 @@ class NodeTypeInitialLanguageTest extends NodeTestBase {
 
+    // Tests if the language field can be rearranged on the manage display tab.
+    $this->drupalGet('admin/structure/types/manage/article/display');
+    $language_display = $this->xpath('//*[@id="language"]');
+    $this->assert(!empty($language_display), 'Language field is visible on manage display tab.');
+    // Tests if the language field is hidden by default.
+    $this->assertFieldByName('fields[language][type]', 'hidden', 'Language is hidden by default on manage display tab.');
+

These tests look good, but can we expand a bit on them to test that the Language field can be shown if we want to.

maxtorete’s picture

Assigned: Unassigned » maxtorete

Ok, I'm on it.

maxtorete’s picture

FileSize
6.36 KB

I have created NodeTypeInitialLanguageTest->testLanguageFieldVisibility() to check if the Language field can be unhidden and it is shown after that.

I thought to create testLanguageFieldVisibility on NodeFieldMultilingualTestCase, but it is Field API related so I think that it doesn't fit there.

maxtorete’s picture

Assigned: maxtorete » Unassigned
Status: Needs work » Needs review
Gábor Hojtsy’s picture

Status: Needs review » Needs work

New test looking very good! Only one more thing from me:

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeTypeInitialLanguageTest.phpundefined
@@ -86,4 +93,35 @@ class NodeTypeInitialLanguageTest extends NodeTestBase {
+    // Creates a node to test if Language field value is shown on node view.
+    $edit = array(
+      'title' => $this->randomName(8),
+      "body[$langcode][0][value]" => $this->randomName(16),
+    );
+    $this->drupalPost('node/add/article', $edit, t('Save'));
+
+    // Loads node page and check if Language field is shown.
+    $node = $this->drupalGetNodeByTitle($edit['title']);
+    $this->assertTrue($node, t('Node found in database.'));
+    $this->drupalGet('node/' . $node->nid);
+    $language_field = $this->xpath('//div[@id=:id]/div', array(
+      ':id' => 'field-language-display',
+    ));
+    $this->assertFalse(empty($language_field), 'Language field value is shown in node page.');

Can we create the node first, check that the field is not visible and then set it to visible and check that it became visible? Sounds like a little change for a much more comprehensive test.

maxtorete’s picture

Assigned: Unassigned » maxtorete

Gábor you read my mind, I was making now that change!

maxtorete’s picture

FileSize
6.68 KB

New patch attached with default settings test in NodeTypeInitialLanguageTest->testLanguageFieldVisibility().

maxtorete’s picture

Assigned: maxtorete » Unassigned
Status: Needs work » Needs review
maxtorete’s picture

FileSize
6.68 KB

Changed assert to assertTrue.

c31ck’s picture

Status: Needs review » Reviewed & tested by the community

Test now includes the recommendations from #32. This looks good to go to me.

webchick’s picture

Status: Reviewed & tested by the community » Fixed
+++ b/core/modules/field/field.api.php
@@ -33,6 +33,8 @@ use Drupal\field\FieldUpdateForbiddenException;
+ *   - visible: The default visibility of the element. Only for 'display'
+ *   context.

(nitpick) "context" should be indented so it's aligned with "visible". Fixed in commit.

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeTypeInitialLanguageTest.php
@@ -86,4 +93,42 @@ class NodeTypeInitialLanguageTest extends NodeTestBase {
+    $this->assertTrue($node, t('Node found in database.'));

No t() on assertions. Fixed in commit.

+++ b/core/modules/node/node.module
@@ -1167,6 +1175,18 @@ function node_view(Node $node, $view_mode = 'full', $langcode = NULL) {
+  // Add language text element on node view if language module is enabled.
+  if (module_exists('language')) {
+    $node->content['language'] = array(
+      '#type' => 'item',
+      '#title' => t('Language'),
+      '#markup' => language_name($langcode),
+      '#weight' => 0,
+      '#prefix' => '<div id="field-language-display">',
+      '#suffix' => '</div>'
+    );
+  }

I stared and stared at this and cannot figure out why it works, but I did confirm that it does by local testing. :) (And of course the test coverage confirms it, too.) I expected to see a #access or a #type switch or something. Huh.

Anyway, committed and pushed to 8.x. Thanks!

As a side-note, I uncovered #1783352: Dragging an item to/from the visible/hidden portion of Field UI does not make it visible/hidden. while testing this patch. Nothing to do with D8MI though. :)

maxtorete’s picture

I stared and stared at this and cannot figure out why it works, but I did confirm that it does by local testing. :) (And of course the test coverage confirms it, too.) I expected to see a #access or a #type switch or something. Huh.

Well, I think that we should add the field if Language module is enabled, and then it will be hidden when the node is rendered, but the Language field is still accesible to hooks or templates.

Now, I think I should have added the Language field on node_build_content so it could be manipulated on hook_node_view and hook_entity_view implementations. I have the patch to this change, should I add a new issue or attach it here and change the status to need review?

maxtorete’s picture

Status: Fixed » Needs review
FileSize
2.68 KB

As I have talked with Gábor Hojtsy, we should show the Language field on manage display even if Language module is not enabled.

I also have moved the field addition to node_build_content so it could be manipulated on hook_node_view and hook_entity_view implementations.

maxtorete’s picture

FileSize
2.67 KB

Old comment updated.

c31ck’s picture

Status: Needs review » Reviewed & tested by the community

This looks good, patch applies cleanly and works as expected. Moving this to node_build_content and making the language field available regardless of language module being active make perfect sense to me.

Gábor Hojtsy’s picture

Agreed, thanks for all the hard work on this!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Gábor Hojtsy’s picture

Issue tags: -sprint

Thanks all!

maxtorete’s picture

Thanks all for reviewing and committing the patch :-)

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