Content language settings cannot be configured for non-translatable types on the one page configuration form

I'd much rather display N/A in place of the checkbox instead of messing with where the columns are. We worked so hard on lining up the columns so you can take a quick look and see what is going on. Not including the translatable column for some while including for others seems to be a problem to me.

One thing I would clearly avoid is using disabled checkboxes, that would be a disaster. Disabled checkboxes are in most cases impossible to tell apart from enabled checkboxes on many systems.

I agree with the beta evaluation - reducing fragility and making things less hacky for contrib should be permitted if the disruption is minimal.

#2143729: Entity definitions miss a language entity key landed.

+++ b/core/modules/language/src/Form/ContentLanguageSettingsForm.php
@@ -68,7 +69,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
-      if (!$entity_type->isTranslatable()) {
+      if (!$entity_type instanceof ContentEntityTypeInterface || !isset($this->entityManager->getBaseFieldDefinitions($entity_type_id)['langcode'])) {
         continue;

So looks like this will need to be updated for the langcode key. @plach / @penyaskito? :)

Like this?

+++ b/core/modules/language/src/Form/ContentLanguageSettingsForm.php
@@ -62,7 +63,8 @@ public function buildForm(array $form, FormStateInterface $form_state) {
-      if (!$entity_type->isTranslatable()) {
+      $langcodeKey = $entity_type->getKey('langcode');
+      if (!$entity_type instanceof ContentEntityTypeInterface || empty($langcodeKey)) {
         continue;
       }

Should there be parantheses? e.g.

+      if (!($entity_type instanceof ContentEntityTypeInterface) || empty($langcodeKey)) {

I do see the Aggregator and File entities in the list.

I don't know how to test the File (I didn't see an edit form for File) but I see language selector on the Aggregator Feed:

As discussed before this does away with all our work to line up things... See #7. I think we should display 'N/A' for translatable when it is not applicable or something similar. Code-wise it looks good.

@penyaskito: can you bring this forward?

Added N/A.

Yay, looks great, thanks @penyaskito.

We use the same N/A in the language summary table for English if English is not possible to translate. So this is very consistent. Needs tests to ensure that eg. File appears on admin/config/regional/content-language both before content translation module and after and then it does not have a checkbox.

Needs works for tests only then :)

I digged that up to make it easy. Looks like the whole method can go away now.

diff --git a/core/modules/aggregator/src/FeedForm.php b/core/modules/aggregator/src/FeedForm.php
index ea6714e..d84c11e 100644
--- a/core/modules/aggregator/src/FeedForm.php
+++ b/core/modules/aggregator/src/FeedForm.php
@@ -7,13 +7,9 @@
 
 namespace Drupal\aggregator;
 
-use Drupal\Component\Utility\String;
 use Drupal\Core\Entity\ContentEntityForm;
-use Drupal\Core\Entity\EntityManagerInterface;
 use Drupal\Core\Form\FormStateInterface;
-use Drupal\Core\Language\LanguageInterface;
 use Drupal\Core\Url;
-use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * Form controller for the aggregator feed edit forms.
@@ -24,21 +20,13 @@ class FeedForm extends ContentEntityForm {
    * {@inheritdoc}
    */
   public function form(array $form, FormStateInterface $form_state) {
-    $feed = $this->entity;
-
-    // @todo: convert to a language selection widget defined in the base field.
-    //   Blocked on https://drupal.org/node/2226493 which adds a generic
-    //   language widget.
-    // Language module may expose or hide this element, see language_form_alter().
-    $form['langcode'] = array(
-      '#title' => $this->t('Language'),
-      '#type' => 'language_select',
-      '#default_value' => $feed->language()->getId(),
-      '#languages' => LanguageInterface::STATE_ALL,
-      '#weight' => -4,
-    );
+    $form = parent::form($form, $form_state);
+    // @todo Allow non translatable entity types having language support to be
+    // configured in the content language setting.
 
-    return parent::form($form, $form_state, $feed);
+    // Ensure the language widget is displayed.
+    $form['langcode']['#access'] = TRUE;
+    return $form;
   }
 
   /**

1. +++ b/core/modules/content_translation/content_translation.admin.inc
   @@ -87,11 +87,16 @@ function _content_translation_form_language_content_settings_form_alter(array &$
   +    $form['settings'][$entity_type_id]['#translatable'] = $entity_type_translatable;
   

   is this used?

2. +++ b/core/modules/content_translation/content_translation.admin.inc
   @@ -139,13 +144,12 @@ function _content_translation_form_language_content_settings_form_alter(array &$
   +  $element = $variables['element'];
      if (!\Drupal::currentUser()->hasPermission('administer content translation')) {
        return;
   ...
   -  $element = $variables['element'];
      $build = &$variables['build'];
   

   moved for no reason

3. +++ b/core/modules/language/src/Form/ContentLanguageSettingsForm.php
   @@ -62,7 +63,8 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   -      if (!$entity_type->isTranslatable()) {
   +      $langcodeKey = $entity_type->getKey('langcode');
   +      if (!$entity_type instanceof ContentEntityTypeInterface || empty($langcodeKey)) {
   

   use $entity->haskey()

Resolving #29/30 and #31. Still needs tests, feel free to work on that. ContentTranslationSettingsTest can be extended to test that the file entity appears for example. and has no checkbox for translation.

So FeedLanguageTest fails on no langcode field where expected...

@penyaskito: wanna keep this going? :)

Back to green.

This should be enough, easier than I thought.

Retirled as a bug. Adding updated screenshots to summary, and updated summary in general.

Looks good, except this tiny thing:

+++ b/core/modules/aggregator/src/Tests/FeedLanguageTest.php
@@ -49,6 +49,15 @@ protected function setUp() {
+    // Enable translation for feeds.
+    $edit['entity_types[aggregator_feed]'] = TRUE;
+    $edit['settings[aggregator_feed][aggregator_feed][settings][language][language_alterable]'] = TRUE;

This is precisely NOT translation but language selection, right? :)

Right, the comment was wrong.

Yay, thanks!

Nice patch, nice test, nice screenshots. Committed a85e59d and pushed to 8.0.x. Thanks!

Thanks for including the beta evaluation in the issue summary.

Amazing, thanks all for your great work.