Entity forms will let users edit different language variants of the same piece of data. Actually only one language variant will be editable at any time so we need a reliable way to know which is the form "active" language. Simply relying on the current content language does not always work as it sometimes makes pretty unconfortable the editing workflow, since the user may want to keep her current language and edit different language variants. Hence we need to be able to explicitly set the active form language through a parameter.

Files: 
CommentFileSizeAuthor
#44 et-form_language-1498874-44.patch12.22 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 41,214 pass(es).
[ View ]
#41 et-form_language-1498874-41.patch13.14 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 41,146 pass(es).
[ View ]
#34 et-form_language-1498874-34.patch11.65 KBplach
PASSED: [[SimpleTest]]: [MySQL] 41,141 pass(es).
[ View ]
#31 et-form_language-1498874-31.interdiff.do_not_test.patch784 bytesplach
#31 et-form_language-1498874-31.patch11.69 KBplach
PASSED: [[SimpleTest]]: [MySQL] 40,582 pass(es).
[ View ]
#28 et-form_language-1498874-28.interdiff.do_not_test.patch3.21 KBplach
#28 et-form_language-1498874-28.patch11.46 KBplach
FAILED: [[SimpleTest]]: [MySQL] 40,563 pass(es), 0 fail(s), and 4 exception(s).
[ View ]
#26 et-form_language-1498874-26.patch10.45 KBpeximo
FAILED: [[SimpleTest]]: [MySQL] 40,565 pass(es), 1 fail(s), and 4 exception(s).
[ View ]
#21 et-form_language-1498874-21.patch3.26 KBpeximo
FAILED: [[SimpleTest]]: [MySQL] 40,546 pass(es), 0 fail(s), and 1,281 exception(s).
[ View ]
#19 et-form_language-1498874-19.patch3.25 KBpeximo
PASSED: [[SimpleTest]]: [MySQL] 40,635 pass(es).
[ View ]
#17 et-form_language-1498874-16.interdiff.do_not_test.patch2.17 KBplach
#17 et-form_language-1498874-16.patch3.09 KBplach
PASSED: [[SimpleTest]]: [MySQL] 40,630 pass(es).
[ View ]
#15 et-form_language-1498874-14.patch3.32 KBpeximo
PASSED: [[SimpleTest]]: [MySQL] 40,633 pass(es).
[ View ]
#10 interdiff.txt1.75 KBattiks
#10 i1498874.patch7.1 KBattiks
FAILED: [[SimpleTest]]: [MySQL] 40,622 pass(es), 0 fail(s), and 128 exception(s).
[ View ]
#8 et-form_language-1498874-8.patch7.09 KBpeximo
PASSED: [[SimpleTest]]: [MySQL] 40,564 pass(es).
[ View ]
#5 et-form_language-1498874-5.patch5.74 KBpeximo
FAILED: [[SimpleTest]]: [MySQL] 40,567 pass(es), 3 fail(s), and 15 exception(s).
[ View ]

Comments

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Issue tags:+language-config

This also concerns multilingual configuration.

Related issue: #1498880: Theme language switcher for seven theme.

there's a mention about forms in #1448330-25: [META] Discuss internationalization of configuration
Symfony has a great formAPI

Also I think the langcode properties/fields affects mostly on render layer of core

Issue tags:+Entity forms

@plach: can you provide an updated version of your thinking on this. I got @vasi1186 to work on #1280996: New language_select element type for form API for language selection generalization. Would be good to find common grounds sooner than later.

Status:Active» Needs review
StatusFileSize
new5.74 KB
FAILED: [[SimpleTest]]: [MySQL] 40,567 pass(es), 3 fail(s), and 15 exception(s).
[ View ]

Here is a first attempt. The patch introduces a new entity form language type and uses it as a default value for the form language if no one is explictly provided to entity_get_form().

Issue tags:+sprint

Status:Needs review» Needs work

The last submitted patch, et-form_language-1498874-5.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new7.09 KB
PASSED: [[SimpleTest]]: [MySQL] 40,564 pass(es).
[ View ]

Fixed negotiation callback for new language type and some tests.

Status:Needs review» Needs work

+++ b/core/modules/entity/entity.module
@@ -12,6 +12,11 @@ use Drupal\entity\EntityStorageException;
+const ENTITY_LANGUAGE_TYPE_FORM = 'entity_language_form';

As discussed with plach we think it makes more sense to have general language type instead of this one. E.g. language_content_edit or something like that, and then use this as default. Let's do so then.

Status:Needs work» Needs review
StatusFileSize
new7.1 KB
FAILED: [[SimpleTest]]: [MySQL] 40,622 pass(es), 0 fail(s), and 128 exception(s).
[ View ]
new1.75 KB

renamed

Status:Needs review» Needs work

@fago and I agreed to move the form language type to the language module. Actually @peximo has already a patch for this but apparently he did not post it...

@plach: that sounds good, makes sense.

However Jose is not convinced that this approach can work for multiple realms, so I think we need to find an alternative not involving a new langauge type. Probably just relying on the content language as a default and relying on Request parameters otherwise.

Assigned:Unassigned» peximo

I'll work as per #13 on an alternative version without a new language type.

Status:Needs work» Needs review
StatusFileSize
new3.32 KB
PASSED: [[SimpleTest]]: [MySQL] 40,633 pass(es).
[ View ]

Removed the new language_form language type and set language_content as form default language.

Status:Needs review» Needs work

Going without a separate language for now is fine to move on I think. As discussed let's keep the default in $form_state['langcode'] initially so the form controller can deal with it properly.

Assigned:peximo» Unassigned
Status:Needs work» Needs review
StatusFileSize
new3.09 KB
PASSED: [[SimpleTest]]: [MySQL] 40,630 pass(es).
[ View ]
new2.17 KB

Addressed #15 since @peximo is working on other stuff, right now.

Status:Needs review» Needs work

Oh, that looks very clean now :)

+++ b/core/modules/entity/lib/Drupal/entity/EntityFormController.php
@@ -208,11 +208,23 @@ class EntityFormController implements EntityFormControllerInterface {
+      // If the site is not multilingual or no translation for the given form
+      // language is available, fall back to the entity language.
+      $langcode = language(LANGUAGE_TYPE_CONTENT)->langcode;
+      $fallback = language_multilingual() ? language_fallback_get_candidates() : array();
+      while (!empty($langcode) && !isset($translations[$langcode])) {

Comment and code doesn't do the same? It uses content language and applies language fallbacks. We should do a final fallback to default entity language though if none of the fallback-languages is in the entity, or?

Status:Needs work» Needs review
StatusFileSize
new3.25 KB
PASSED: [[SimpleTest]]: [MySQL] 40,635 pass(es).
[ View ]

Comments fixed as per fago's suggestion. Actually we already fall back to the entity language.

Status:Needs review» Reviewed & tested by the community

Talked with @fago and he's ok with the latest patch.

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new3.26 KB
FAILED: [[SimpleTest]]: [MySQL] 40,546 pass(es), 0 fail(s), and 1,281 exception(s).
[ View ]

getFormLangcode() should return a langcode not a language object. Fixed.

Status:Needs review» Needs work

The last submitted patch, et-form_language-1498874-21.patch, failed testing.

We need tests for this, moreover the fix won't work if Language module is disabled. We need to fix the Entity::language() method to always return a valid language object.

@peximo: are you continuing work on this issue? It is a very important part of making Drupal 8 multilingual! Thanks for working on it so far!

Assigned:Unassigned» peximo

@gabor: I'll work on this issue tomorrow morning.

Status:Needs work» Needs review
StatusFileSize
new10.45 KB
FAILED: [[SimpleTest]]: [MySQL] 40,565 pass(es), 1 fail(s), and 4 exception(s).
[ View ]

Fixed Entity::language() method and added some tests.
@gabor: plach thinks is better to do the test with the entity_test form controller (see #1757232: Improve test coverage for the entity form controller). So we might need to merge the two issues.

Status:Needs review» Needs work

The last submitted patch, et-form_language-1498874-26.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new11.46 KB
FAILED: [[SimpleTest]]: [MySQL] 40,563 pass(es), 0 fail(s), and 4 exception(s).
[ View ]
new3.21 KB

Sorry, we were not taking entity language changes into consideration. Rerolled the patch with that fixed plus a couple of minor adjustments.

Title:Provide language awareness to forms (introduce the form language concept)Provide language awareness to entity forms (introduce the form language concept)
Issue tags:-language-config

Retitling and untagging since we are not trying to handle form language in a generic fashion as per the discussion with @Jose.

Issue summary:View changes

Updated issue summary.

Status:Needs review» Needs work

The last submitted patch, et-form_language-1498874-28.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new11.69 KB
PASSED: [[SimpleTest]]: [MySQL] 40,582 pass(es).
[ View ]
new784 bytes

Actually there may be cases where the entity langcode is not set, hence notices are thrown.

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

The last submitted patch, et-form_language-1498874-31.patch, failed testing.

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

No failures on my box.

#31: et-form_language-1498874-31.patch queued for re-testing.

StatusFileSize
new11.65 KB
PASSED: [[SimpleTest]]: [MySQL] 41,141 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

I'm not sure I fully understand why this is necessary (issue summary is a bit sparse), but all of you seem to agree that it is a good idea. I hope you guys asked each other some criticizing questions in the discussions ;)

The code in EntityFormController::getFormLangcode() could be improved for monolingual sites and clarified in terms of code logic, but that is pretty minor and can be done in a later follow-up.

This deals with the rapidly changing entity landscape and is at a definite risk of commit conflicts.

#34: et-form_language-1498874-34.patch queued for re-testing.

This looks great. just one question:

+++ b/core/modules/entity/lib/Drupal/entity/Tests/EntityTranslationTest.phpundefined
@@ -22,7 +22,7 @@ class EntityTranslationTest extends WebTestBase {
-  public static $modules = array('entity_test', 'locale');
+  public static $modules = array('entity_test', 'locale', 'node');

Why not adding multilingual forms to the entity_test module?

@catch: the entity_test module is a pure API test module, it does not have page callbacks or forms at all. I think it would be important at this point to get this moving instead of pushing it back to implement a UI for testing in entity_test module while we already have the necessary stuff to test elsewhere, such as in node module.

Status:Reviewed & tested by the community» Needs work

OK that's fine, but in that case let's add a new test class for the forms, so we don't need to install node module for the other test methods.

Status:Needs work» Needs review
StatusFileSize
new13.14 KB
PASSED: [[SimpleTest]]: [MySQL] 41,146 pass(es).
[ View ]

Moved it to a separate file with only the setup routines required for this one.

Status:Needs review» Reviewed & tested by the community

Should be back to RTBC then.

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/entity/lib/Drupal/entity/Tests/EntityTranslationTest.phpundefined
+++ b/core/modules/entity/lib/Drupal/entity/Tests/EntityTranslationTest.phpundefined
@@ -22,7 +22,7 @@ class EntityTranslationTest extends WebTestBase {
@@ -22,7 +22,7 @@ class EntityTranslationTest extends WebTestBase {
    *
    * @var array
    */
-  public static $modules = array('entity_test', 'locale');
+  public static $modules = array('entity_test', 'locale', 'node');

This change shouldn't be needed then?

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new12.22 KB
PASSED: [[SimpleTest]]: [MySQL] 41,214 pass(es).
[ View ]

Indeed, we should not have any business touching that file anymore.

Status:Reviewed & tested by the community» Fixed

Thanks! Committed/pushed to 8.x.

I don't think this really needs change notice so moving straight to fixed, but please re-open/write one if you disagree.

I think we don't need a change notice here: this ended-up being a far less APIsh change than I originally thought.

Issue tags:-sprint

This should be done for our sprint then :) Thanks all!

Status:Fixed» Closed (fixed)

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

Issue summary:View changes

Updated issue summary.