Field UI creates fields that can never be translated

+++ b/core/lib/Drupal/Core/Field/FieldConfigBase.php
@@ -115,11 +115,11 @@
    * Flag indicating whether the field is translatable.
...
-   * Defaults to TRUE.
+   * Defaults to FALSE.
...
-  public $translatable = TRUE;
+  public $translatable = FALSE;

Why not make this configurable when field is created?

@plach: I believe the UI problem shown in #8 is preexisting and is not in scope here neither is due to this patch right? So I don't think we should be concerned with that.

Fix tags.

The related issue changes the forms

Why the prio change?

This completely breaks the ability to have translated fields, without a workaround other than manually editing config files. Sounds critical to me.

@plach: ok I meant that the entity is not enabled for translation while the fields are; what would be a bad side effect of making them still default to TRUE for their configuration? sounds like that would still be ignored unless the entity bundle is also configured to be translatable?

That sounds like less of a change compared to how it works currently, so fine with me :)

There's a separate issue for default values for fields. So as bug this RTBC
I just wonders why form hardcodes default values when their defaults should be taken from field entities.

+++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
@@ -223,11 +223,13 @@ public function buildForm(array $form, FormStateInterface $form_state, $entity_t
+    // modules can form_alter() the value for newly created fields. By default
+    // we create field storage as translatable so it will be possible to enable
+    // translation at field level.
...
-      '#value' => FALSE,
+      '#value' => TRUE,

Looks good to me.

1. +++ b/core/modules/field_ui/src/Form/FieldStorageAddForm.php
   @@ -223,11 +223,13 @@ public function buildForm(array $form, FormStateInterface $form_state, $entity_t
   -    // Place the 'translatable' property as an explicit value so that
   -    // contrib modules can form_alter() the value for newly created fields.
   +    // Place the 'translatable' property as an explicit value so that contrib
   +    // modules can form_alter() the value for newly created fields. By default
   +    // we create field storage as translatable so it will be possible to enable
   +    // translation at field level.
        $form['translatable'] = array(
          '#type' => 'value',
   -      '#value' => FALSE,
   +      '#value' => TRUE,
        );
   

   So this is the key change in the patch. I looked into what @andypost asked and there does not seem to be a field storage config instance yet to take default values from. We could create one to init this value but then would throw that away later. That may be better to keep consistency.

2. diff --git a/core/modules/node/src/Tests/NodeAccessLanguageAwareCombinationTest.php b/core/modules/node/src/Tests/NodeAccessLanguageAwareCombinationTest.php
   index f73ac3b..d0e1ee2 100644
   @@ -58,7 +58,6 @@ protected function setUp() {
          'entity_type' => 'node',
          'type' => 'boolean',
          'cardinality' => 1,
   -      'translatable'  => TRUE,
          'settings' => array(
            'on_label' => 'Private',
            'off_label' => 'Not private',
   @@ -69,6 +68,7 @@ protected function setUp() {
   
   @@ -69,6 +68,7 @@ protected function setUp() {
        entity_create('field_config', array(
          'field_storage' => $field_storage,
          'bundle' => 'page',
   +      'translatable'  => TRUE,
          'widget' => array(
            'type' => 'options_buttons',
          ),
   

   Given that the defaults are now both true for storage and field config, neither the removed, neither the added translatable booleans are needed changes, right? At least the added ones don't need to be added, since it still defaults to TRUE? Not having those extra things in the patch would make it much simpler to review.

1. Yes, the thing is that at this point, we do not yet know if we will create a new storage or re-use an existing one. However, it is possible that the refactoring that @amateescu is working on in #552604: Adding new fields leads to a confusing "Field settings" form will change that.

ALl changes look good to me and my concerns are addressed. The test cleanups are good. No need to set things to something they are already set to :D

Looks all good to get in.

Committed 942df46 and pushed to 8.0.x. Thanks!

Thanks for completing the beta evaluation.

Yay, thanks!

@plach++