Split from #2075095-14: Widget / Formatter methods signatures needlessly complex :

Entity forms are structured with field widgets nested under langcode keys:
$form['body']['fr'] = ...
$form['field_image']['und'] = ...

Those langcode subkeys are basically useless now, and do not determine in which langode the submitted values are stored: WidgetBase::extractFormValues() assigns values into the $items object that gets passed to it, how the form is structured doesn't change a thing.

- We should get rid of those langcode subkeys in the $form structure, we're not editing entities in multiple languages in the same form, core entity form controllers edit an entity in one single language. Within that language, some fields are translatable and some are not, that's it. It doesn't make a difference for the form structure.
- Even if some contrib module wanted to do "side editing in two languages in one form", it's then its responsibility to structure those forms accordingly.

At any rate, it's not the job of *widgets* to spit form $elements nested below a langcode subkey, as they do currently (see the end of WidgetBase::form()). Widgets generate "what it takes to edit this field", that's it. Multilingual structure, *if needed*, should be added on top of the result of $widget::form() by the calling code.

To which @plach replied:

Totally +1 on removing the langcode level in the form widget structures, it has always been a major source of troubles. I agree that for advanced use cases the implementing module should build its form(s) by assembling form widgets or possibily entire entity forms with their respective languages, as suggested in #1728816: Ensure multiple entity forms can be embedded into one form

Motivations:
Having the langcodes structure $form / $form_state when they are not needed:
- means chances of inconsistencies and weird behavior
- makes it vastly painful to change the language assignment logic if we need to (see #2076445: Make sure language codes for original field values always match entity language regardless of field translatability). Language assignment is internal, it should stay internal.

Patch summary:
- Renames $form[$field_name][$langcode] to $form[$field_name]['widget'] in entity forms.
(see #10 item 2 for why we still need a separate sub-element below $form[$field_name])
- Removes the $langcode param in field_form_(get|set)_state(). Those functions are used internally to structure the processing data that each field places into $form_state and avoid clashes. Just like for $form, we don't need $langcode to be part of the default structure of $form_state. The code that builds the form can provide the additional structure it needs, if it needs one.

API changes:
- Change of structure in entity $forms:
code doing form_alter() on those needs to be updated (see forum_form_node_form_alter())
tests doing drupalPost() / drupalPostForm() with field values on entity forms need to be updated
- field_form_(get|set)_state() lose their $langcode param.

Files: 
CommentFileSizeAuthor
#18 field_form_langcode-2083811-18-FULL.interdiff.do_not_test.patch3.56 KBplach
#18 field_form_langcode-2083811-18-FULL.patch234.42 KBplach
PASSED: [[SimpleTest]]: [MySQL] 59,242 pass(es).
[ View ]
#15 field_form_langcode-2083811-15-REVIEW-do-not-test.patch23.32 KByched
#15 field_form_langcode-2083811-15-FULL.patch234.11 KByched
FAILED: [[SimpleTest]]: [MySQL] 59,171 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#13 field_form_langcode-2083811-13-REVIEW-do-not-test.patch23.32 KByched
#13 field_form_langcode-2083811-13-FULL.patch233.42 KByched
PASSED: [[SimpleTest]]: [MySQL] 58,974 pass(es).
[ View ]
#13 interdiff.txt24.48 KByched
#12 field_form_langcode-2083811-11.patch220.88 KByched
PASSED: [[SimpleTest]]: [MySQL] 58,855 pass(es).
[ View ]
#12 interdiff.txt3.3 KByched
#10 field_form_langcode-2083811-10.patch217.58 KByched
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#10 interdiff.txt3.03 KByched
#8 field_form_langcode-2083811-8.patch216.98 KByched
PASSED: [[SimpleTest]]: [MySQL] 58,900 pass(es).
[ View ]
#6 field_form_langcode-2083811-6.patch208.17 KByched
FAILED: [[SimpleTest]]: [MySQL] 57,853 pass(es), 155 fail(s), and 31 exception(s).
[ View ]
#3 field_form_langcode-2083811-3.patch158.88 KByched
FAILED: [[SimpleTest]]: [MySQL] 56,333 pass(es), 359 fail(s), and 127 exception(s).
[ View ]
#1 field_form_langcode-2083811-1.patch32.21 KByched
FAILED: [[SimpleTest]]: [MySQL] 52,621 pass(es), 1,292 fail(s), and 217 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new32.21 KB
FAILED: [[SimpleTest]]: [MySQL] 52,621 pass(es), 1,292 fail(s), and 217 exception(s).
[ View ]

Alright. Let's see what breaks :-)

Status:Needs review» Needs work

The last submitted patch, field_form_langcode-2083811-1.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new158.88 KB
FAILED: [[SimpleTest]]: [MySQL] 56,333 pass(es), 359 fail(s), and 127 exception(s).
[ View ]

Most. Boring. Patch. Ever.
No interdiff, don't bother reviewing until this is green :-)

Side note: the mere amount of tests that had to deal with that, and needed to "use Drupal\Core\Language\Language" just to get a Language::LANGCODE_NOT_SPECIFIED to put in their drupalPost() data, shows it's a huge win...

Status:Needs review» Needs work

The last submitted patch, field_form_langcode-2083811-3.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new208.17 KB
FAILED: [[SimpleTest]]: [MySQL] 57,853 pass(es), 155 fail(s), and 31 exception(s).
[ View ]

More fixes...

Status:Needs review» Needs work

The last submitted patch, field_form_langcode-2083811-6.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new216.98 KB
PASSED: [[SimpleTest]]: [MySQL] 58,900 pass(es).
[ View ]

Status:Needs review» Needs work

Awesome cleanup! We should now be able to get rid of the EntityFormController::submitEntityLanguage() method too.

  1. +++ b/core/modules/field/field.deprecated.inc
    @@ -488,10 +488,9 @@ function field_read_instances($conditions = array(), $include_additional = array
      *   // The '#language' key holds the same value of $langcode and it is used
      *   // to access the field sub-array when $langcode is unknown.

    Does this need to be updated?

  2. +++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetBase.php
    @@ -130,17 +112,31 @@ public function form(EntityInterface $entity, $langcode, FieldInterface $items,
    +        'wrapper' => $elements,

    Why are we replacing the langcode level with a static wrapper?

  3. +++ b/core/modules/field/lib/Drupal/field/Tests/FieldTestBase.php
    @@ -8,8 +8,10 @@
    +

    Surplus blank line?

  4. +++ b/core/modules/field/lib/Drupal/field/Tests/FieldUnitTestBase.php
    @@ -138,14 +139,14 @@ function _generateTestFieldValues($cardinality) {
    + * @param $langcode
    + * The language code for the values.
    * @param $column
    * (Optional) the name of the column to check.
    */
    - function assertFieldValues(EntityInterface $entity, $field_name, $langcode, $expected_values, $column = 'value') {
    + function assertFieldValues(EntityInterface $entity, $field_name, $expected_values, $langcode = Language::LANGCODE_NOT_SPECIFIED, $column = 'value') {

    Missing (optional) + description of the default value.

    Moreover, given that the $langcode argument is being passed to EntityInterface::getTranslation(), it would more appropriate to default to Language::LANGCODE_DEFAULT.

Status:Needs work» Needs review
StatusFileSize
new3.03 KB
new217.58 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

@plach: Thanks for the review :-)

1. Right, fixed
2. Look at the structure of what WidgetBase::form() returns:
$form[$field_name] is a '#type' => 'container'. So we cannot put the widget (e.g. '#type' => 'select') directly at that level, we need a separate wrapper. The langcode level used to do that - we don't need it to be the langcode, and we don't need it to be reflected in the structure of the submitted values, but we still need a separate wrapper level in the $form.
3. Fixed.
4. Doc fixed. All the code that currently calls assertFieldValues() in HEAD (where the argument is not optional) passes Language::LANGCODE_NOT_SPECIFIED - so I kept that ?

Note that I'm not fully done yet, field_form_[set|get]_state() needs its $langcode param removed too.
I'll try to provide a 'do-not-test' patch with the non-test changes only (those are just a few KBs) for easier review.

I'll try to provide a 'do-not-test' patch with the non-test changes only (those are only a few KBs) for easier review.

Wonderful :)

Please remember to remove EntityFormController::submitEntityLanguage(): its sole purpose was updating form state field language codes after an entity language change.

StatusFileSize
new3.3 KB
new220.88 KB
PASSED: [[SimpleTest]]: [MySQL] 58,855 pass(es).
[ View ]

[edit: crosspost with #11 :-)]
Missed the part about submitEntityLanguage() - yes it does look like it's stale now, let's see what happens without it.

StatusFileSize
new24.48 KB
new233.42 KB
PASSED: [[SimpleTest]]: [MySQL] 58,974 pass(es).
[ View ]
new23.32 KB

Updated patch:
- renamed $form[$field_name]['wrapper'] to $form[$field_name]['widget'], for clarity - the "wrapper" is $form[$field_name] itself.
- removed the $langcode argument in field_form_(get|set)_state()
See interdiff.

As mentioned in #10, attached are:
- a 'REVIEW-do-not-test' patch with just the actual code changes for review
- a 'FULL' patch with the test updates (90% of the patch size...)

Priority:Normal» Major
Issue tags:+API change, +Field API

Updated the OP with patch summary & API change sections.

Adding tags, and raising to major: having the langcodes structure $form / $form_state when they are not needed
- means chances of inconsistencies and weird behavior
- makes it vastly painful to change the language assignment logic if needed (see #2076445: Make sure language codes for original field values always match entity language regardless of field translatability). Language assignment is internal, it should stay internal.

Issue summary:View changes

summary & API changes

Issue summary:View changes

motivations

StatusFileSize
new234.11 KB
FAILED: [[SimpleTest]]: [MySQL] 59,171 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
new23.32 KB

Status:Needs review» Reviewed & tested by the community

This makes me feel so good :D

RTBC if green, otherwise the following nits could be fixed:

+++ b/core/modules/field/field.form.inc
@@ -209,8 +205,8 @@ function field_add_more_js($form, $form_state) {
+function field_form_get_state($parents, $field_name, &$form_state) {
@@ -231,8 +225,8 @@ function field_form_get_state($parents, $field_name, $langcode, &$form_state) {
+function field_form_set_state($parents, $field_name, &$form_state, $field_state) {
@@ -243,30 +237,14 @@ function field_form_set_state($parents, $field_name, $langcode, &$form_state, $f
+function _field_form_state_parents($parents, $field_name) {

These could use array type-hints as the PHP docs.

Issue summary:View changes

fix formatting

Issue summary:View changes

grammar

Issue summary:View changes

reword

Issue summary:View changes

drupalPostForm()

Status:Reviewed & tested by the community» Needs work

The last submitted patch, field_form_langcode-2083811-15-FULL.patch, failed testing.

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new234.42 KB
PASSED: [[SimpleTest]]: [MySQL] 59,242 pass(es).
[ View ]
new3.56 KB

Just a typo in the Forum test. Also fixed #16.

Title:Remove langcode nesting for fields in entity $form structuresChange notice: Remove langcode nesting for fields in entity $form structures
Status:Reviewed & tested by the community» Active
Issue tags:+DX (Developer Experience), +Needs change record, +Approved API change

OMG. I approve this sooooo effing hard.

101 files changed, 598 insertions, 903 deletions.

Lovely diffstat!

+++ b/core/modules/block/custom_block/lib/Drupal/custom_block/Tests/CustomBlockCreationTest.php
@@ -49,9 +48,8 @@ protected function setUp() {
     // Create a block.
     $edit = array();
-    $langcode = Language::LANGCODE_NOT_SPECIFIED;
     $edit['info'] = $this->randomName(8);
-    $edit["block_body[$langcode][0][value]"] = $this->randomName(16);
+    $edit['block_body[0][value]'] = $this->randomName(16);
     $this->drupalPostForm('block/add/basic', $edit, t('Save'));

LOVELY code sample!!

OMG, this is seriously one of my favourite patches of all time!! :D :D

Committed and pushed to 8.x. YEAAAAHH!!!

Will need a lovely, lovely change notice. :D

Title:Change notice: Remove langcode nesting for fields in entity $form structuresRemove langcode nesting for fields in entity $form structures
Status:Active» Fixed
Issue tags:-Needs change record

Thanks @webchick, glad that you like it so much ;-)

Created two separate change notices:
https://drupal.org/node/2088511
https://drupal.org/node/2088515

@yched++

Change notices look good to me. I just performed a very minor adjustment to https://drupal.org/node/2088511.

The change caused the content translation totally broken #1946462-52: Convert content_translation_translatable_form() to the new form interface

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

Issue summary:View changes

API change