Language setup for entity and field based rendering in views is independent, confusing UI, lacking test coverage

For Field UI fields, the code for figuring out which language to use to display the field is centralized on
\Drupal\field\Plugin\views\field\Field::field_langcode()
and the method on that same class that actually does the translation of the entity is
\Drupal\field\Plugin\views\field\Field::process_entity().

field_langcode() already has a fallback for not translating, or LanguageInterface::LANGCODE_NOT_SPECIFIED, but it is not currently exposed in the UI. So that will be easy to add.

We will also need to add to the UI options for "the row's language" and "the page's language" (or whatever we want to call them in the UI in the Language settings on the View display).

For Entities, I think all we'd need to add is the static language settings from the Field Language setting options we currently have. These could use a "fixed language" renderer, with an option to tell it what language to use. That shouldn't be too difficult to manage.

Right, making these options actually *work* for Fields is the Hard Part of this issue, and I think you're right that the "language of the row" option is the only one that is problematic for Fields.

The language code value should be available though:
- If a view is of entities, each row is a translation, so the "row's language" would be (entity data table).langcode. All the entity data tables have this field now.
- If a view is of entity revisions, each row is a translation of a revision, so the "row's language" would be (entity revision data table).langcode. We can probably use that anyway though because if the view is not revision-based, then the data table and revision data table are presumably joined, right? Some entities don't have revisions though; in that case stick with the entity data table.

Actually, TranslationLanguageRenderer... I am not sure it is using the right logic here. It is trying, in this order: the data table, the revision table, and the base table. I think it should be using the revision data table, the data table, the revision table, and the base table, in that order. Right?

Anyway, yes fields will need to do something similar to make sure the correct langcode field is available in the query and extract it... Is it possible we can make field rendering use the existing entity renderer classes, rather than rewriting the logic?

There's another discussion about changing the field rendering pipeline for Views on
#1867518: Leverage entityDisplay to provide fast rendering for fields
which I just marked as related to this issue. Not sure exactly how related, but it seems that they'd at least impact each other.

OK good, you closed the other issue.

So. Two things to fix/discuss:

a) Machine/internal names of the options. For the entities, the machine names of the options are things like
translation_language_renderer, set in

And then they are used like this, in EntityRow::getRenderer():

      $class = '\Drupal\views\Entity\Render\\' . Container::camelize($this->displayHandler->getOption('rendering_language'));
      $this->renderer = new $class($this->view, $this->languageManager, $this->entityType);

So the machine names are basically underscore-separated decamelized names of classes that need to be in the \Drupal\views\entity\Render namespace. This is really kind of ... lame? bad coding? to say the least, and this scheme for figuring out which class to use isn't going to work if we have all those other options that we have on the fields, because there's no way we're going to make a specific class for each specific-language option, for instance.

So. IMO there is no point to trying to preserve the same internal names that the entity rendering is using for the options it has now, because we're going to have to change those two lines of code anyway to do something else to figure out what class to use and how to instantiate it.

Given that, I think we should adopt the machine names that the field UI is using. Those are coming from Drupal\views\Plugin\views\PluginBase::listLanguages(), and include:

a) Specific languages - just uses the two-letter language code. Also listed here are Not specified, Not applicable, and those have their three-letter language codes.

b) ***LANGUAGE_(something)*** -- includes ***LANGUAGE_site_default*** as well as **LANGUAGE_(type ID)*** for each of the negotiated language types (content, UI text, and anything else that exists). This will take care of the option on entities that is currently called "Current language", which really means "selected Content language". Personally I think the Field labels for this are better than "Current language" -- who knows what that means. And by the way these options currently say "Language selected for ..." not "Language negotiated for..." in the language choices for Field language. See the listLanguages() method code.

c) Then we will need to add two more options here, from the entity:
- "Default language", which really means "The entity's default language"
- "Translation language", which really means "The language of this Views row"

So my opinion is that we "just" need to:

(1) Figure out good internal/machine codes for these two options -- which I think should probably be ***LANGUAGE_entity_default*** and ***LANGUAGE_entity_translation***

(2) Figure out good labels for these two options, which I really don't know... Maybe "Translated language of content" and "Default language of content", so that the complete list would be:

- Translated language of content [I think this one should be first and should be the default, because that means "the row's language"]
- Default language of content
- Site's default language (English)
- English
- .... all other languages one by one ...
- Not specified [Do we actually want this?]
- Not applicable [Do we actually want this?]
- Language selected for User interface text
- Language selected for Content

(3) Add these two new language options to the listLanguages() method (and define a new flag PluginBase::INCLUDE_ENTITY that means "include these two new entity row language options only if this flag is set to TRUE). We'll also need to look at PluginBase::queryLanguageSubstitutions() to see if we need to do something for the entity languages... which I think we would because languages and all other fields can be added as Group By and then they have to get into the query, sigh.

(4) Make the options list for the Display language call listLanguages() withPluginBase::INCLUDE_ENTITY, as well as \Drupal\Core\Language::STATE_ALL (or maybe just STATE_CONFIGURABLE -- would that get rid of Not specified and Not applicable?), \Drupal\Core\Language::STATE_SITE_DEFAULT, and \Drupal\views\Plugin\views\PluginBase::INCLUDE_NEGOTIATED, so that all the options are listed.

(5) Modify those two lines in EntityRow::getRenderer() so that it uses these new codes and gets an appropriate class, instead of assuming the machine/internal name *is* the decamelized name of the class. We may need to add a new class that does specific languages, and then in those two lines we'd instantiate the class and set the language -- that one would be used for Site Default as well as the specific language options, and maybe we could also use it for the two negotiated/selected languages, because we have a method PluginBase::queryLanguageSubstitutions() that will get the actual language to be used for the LANGUAGE_* options now.

(6) Figure out how to make the Field renderer work for those additional two options. [I think this is actually the main hard part!]

RE #9 - Agreed that "Translated language of content" and "Language selected for content" are confusing from the options in #7/8.

Could we make the last two something like
Page language selected by Content methods
Page language selected by User interface text methods

And maybe the first two could be:
Content language of views row
Default language of content in views row

I think that would sufficiently disambiguate the options, and make it clearer what they really mean?

The UI words "methods" and "selected" are mentioned on the Detection and Selection admin page.

The "row" word is not shown much in the Views UI, but it is used in several places:
- The title of the row style dialog is "[display name]: How should each row in this view be styled" and the help text under the option radios is "You may also adjust the settings for the currently selected row style."
- The settings page dialog for the row style - title is "[display name]: Row style options".
- There at places like the Header text areas where you have an option like "Use replacement tokens from the first row".

So I think both of these would be decent choices. Thoughts?

I also looked through the patch. Except for making the Field renderer work for the two new special languages, which right now just has a placeholder and @todo in your patch, it all looks fairly reasonable to me.

I still think that we should consider adding the two new special languages to the centralized listLanguages method on PluginBase though -- when I set that up on another issue, I consolidated all of the list language functions there -- that was the idea of that method, so we didn't have a bunch of different places in Views all making their own ad-hoc lists of languages with different UI choices. I really do think it's better to centralize the whole thing there.

Anyway if I have some free time during the holidays or possibly later today, I might try to pick this up, but it sure looks like an excellent start!

OK. Here's a bit of work on this:

a) Added the new special languages to PluginBase::listLanguages so the listing of languages is still centralized. I added a note to the documentation that these languages are only for display options, meaning they won't work in query substitutions. This should be fine, because we don't want these options on filters and arguments (wouldn't make sense there anyway).

b) Updated the UI names for the special languages as in #11, plus or minus a typo. Screen shot:

c) Fixed test failure of #10 (the problem was the default value of language had changed) and hopefully anticipated some other test failures due to (b). The test that failed in#10, ViewEditTest in Views UI module, at least passes locally now.

Hopefully this is progress. ;)

Doh. I just realized that the 2nd option "Default language of content in view row" should be "Original language of content in view row".

The testbot failed on that last patch, not the patch by the way.

Anyway, no sense retesting. Here's a new patch with just that one change (it's in PluginBase::listLanguages() ).

Thank you for working on it!

1. +++ b/core/modules/field/src/Plugin/views/field/Field.php
   @@ -916,13 +912,9 @@ protected function addSelfTokens(&$tokens, $item) {
   -  function field_langcode(EntityInterface $entity) {
   

   Just curios, does it make sense to camelcase it at the same time?

2. +++ b/core/modules/field/src/Plugin/views/field/Field.php
   @@ -939,6 +931,26 @@ function field_langcode(EntityInterface $entity) {
      /**
   +   * Resolve the language code to be used for fields.
   +   *
   +   * @return string
   +   *   Language code to be used.
   +   */
   +  protected function field_langcode() {
   

   What about using fieldLangcodeConfigured, so its clear where its coming from?

3. +++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
   @@ -215,11 +215,11 @@ public function initDisplay(ViewExecutable $view, array &$display, array &$optio
   -    // Convert the field_langcode and field_language_add_to_query settings.
   -    $field_langcode = $this->getOption('field_langcode');
   +    // Convert the rendering_language and field_language_add_to_query settings.
   +    $field_langcode = $this->getOption('rendering_language');
        $field_language_add_to_query = $this->getOption('field_language_add_to_query');
        if (isset($field_langcode)) {
   -      $this->setOption('field_langcode', $field_langcode);
   +      $this->setOption('rendering_language', $field_langcode);
          $this->setOption('field_langcode_add_to_query', $field_language_add_to_query);
          $changed = TRUE;
        }
   

   ... Isn't that some sort of BC code which should be better dropped?

The patch still is not working for Field views.

Regarding camel case, sure.

Also I am not sure the first and last options are easy to tell apart for anyone who is not a pro?!

Given that by default Content language inherits from UI language, why don't we check whether we have only one configurable language type and in that case we provide just a single option, like "Page language"? This would make the default scenario easier for non-pro. And if someone managed to provide separate configurations for those we can safely assume she is familiar with the language negotiation concepts. To sum up:

-Simple case:
Page language

-Advanced case:
Language selected for page User Interface
Language selected for page Content

Page language selected by Content methods
Page language selected by User interface text methods

These are misleading: negotiation methods can be the same for both language types, although being inspected in different order. The difference is (may be) in the negotiated languages that are assigned to the various language types.

Good ideas, although I think the logic to implement "see how many there are and modify the displayed name accordingly" would be annoying to add to the listLanguages() function? Maybe it's only a line or two though.

Anyway... Besides agreeing on the names, this patch still needs to be added to: right now it does not actually contain the code to make the new options work for field-based views.

Good ideas, although I think the logic to implement "see how many there are and modify the displayed name accordingly" would be annoying to add to the listLanguages() function? Maybe it's only a line or two though.

Here it is:

$types = \Drupal::languageManager()->getLanguageTypes();
if (count($types) == 1) {
  // Simple
}
else {
  foreach ($types as $type) {
    // Pro
  }
}

Looking good! So we just need to address this @todo now?

+  protected function fieldLangcodeConfigured() {
+    $langcode = $this->view->display_handler->options['rendering_language'];
+    $substitutions = static::queryLanguageSubstitutions();
+    $substitutions += array(
+      // @todo Backwards compatible fallback, needs to be implemented.
+      '***LANGUAGE_entity_translation***' => $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_CONTENT),
+      '***LANGUAGE_entity_default***' => LanguageInterface::LANGCODE_DEFAULT,

now?

// @todo Backwards compatible fallback, needs to be implemented.

Sorry, I'm not sure I get what this is supposed to mean. AFAICT the only way to obtain language fallback efficiently is loading a full entity and calling EntityManagerInterface::getTranslationFromContext() on it. If I'm not mistaken this is what already happens when rendering then entity in the current content language. What else do we need?

Basically what that means is that Gábor put blatantly wrong placeholder code in there and it needs to be replaced by real functionality. :)

We're adding new capability here: fields will be able to be displayed in the language of the row, or in the "original" translation language of the entity. This will make field views work like entity views, so that all the language UI in Views is uniform, independent of whether you are doing a field view or an entity view. But currently fields do not know how to display using "row language" or "original language of the entity", so that part needs to be written.

Looks like we are changing how we store language information in the config entities as a part of this fix, so this is an upgrade path issue.

Working on this

I have some working code reusing language renderers but the patch is not ready yet, I'll post it tomorrow.

Here is the patch. It is far from complete but manual testing results are promising.

Things still missing:

• Additional test coverage.
• Removal of the field_langcode_add_to_query option, since it's basically applying language conditions for field tables, which now are joined to the data table and match its language. This is basically equivalent to what we are doing in the translation renderer, aside from trying to support fallback at query level which is tricky.
• Renaming of a few methods: ::fieldLangcodeAvailable() should be called ::getFieldLangcodeAvailable() and so on.
• Discussing the language list a bit more: I still find its options a bit confusing, I'll expand on this in a following post.

Btw, I found #2414721: EntityAdapter should be instantiated per language while testing this...

The trait solution with the accessor methods is interesting but adding yet one more service which would need the other services injected again may be more trouble, since we have all details and services present in the existing handlers.

Not sure I get what you mean, can you expand on this?

Ok, so you are suggesting to keep the trait, right? This is the part I'm not completely clear on :)

Yep, I'm not totally convinced about it too, composition would sound as the go-to choice here, but yeah, it's easy and works so for now I guess it's ok.

This should be green again.

Removed the field_langcode_add_to_query option. Let's see what happens.

The correct slim interdiff

Just took a look at the latest patch...

A question in Entity/Render/DefaultLanguageRenderer.php:

+  public function getLangcode(ResultRow $row) {
     return $row->_entity->getUntranslated()->language()->getId();
   }

So I realize this is not a code change, since this line was the same before the patch... but ... Some view rows may have more than one entity in them (relationships! Such as a view containing fields from a node and a taxonomy term and an author). So... ???? will this work? Actually, when we have a relationship in a multilingual view, if both entities being related have translations, does the join match the languages? I guess this is a separate issue but man, that's scary code!

Hm. Took a look at the ResultRow class, and ... it's a bit lacking in details, but it looks like it's returning the main entity for the row. So if we're rendering one of the other entities in the relationships, will this be the right language? Seems wrong.

The same $row->_entity happens in the DefaultLanguageRenderer too.

Thoughts? Have we tested this with relationships?

RE #48: I guess this wouldn't have come up before, because a view using Entity View Mode rendering would only have the main entity to render. So this usage of $row->_entity would work for that. But I don't think we can count on it working for Field based views, which could be displaying fields from several different entities.

I was going to suggest a fix for the case of the "untranslated" option anyway... but the more I think about it, the more I think that at least 99.999% of the time you'd want to display all entities in a row in the same language.

So actually I think that we should go with the current patch. This would mean that technically, in the case of relationships, the option "Display in language of row" or whatever it's called would really mean "Display everything in the language of the main entity of the row", and similar for the "display in base language" option.

It remains to be seen whether joins/relationships are working completely as we want them to, but that is definitely a different issue. So yes, let's move forward with this!

This performs the additional code clean-up we were mentioning earlier (and some more).

As I was saying above, I'm still not happy with the language selection list. If we compare it with the one appearing in the content language settings page, we can notice various inconsistencies:

1. Dynamic languages should be listed first, but current languages are the last options.
2. We have different wording in the two cases (should we be using common code to generate those lists?).
3. Only interface language is listed there.

Therefore I'd be in favor of moving current languages up, after the default language, unify wording and apply my suggestion from #19, i.e. list only configurable languages.

This is definitely not entity render caching. Views' "field views" render pipeline must be doing (static) caching somewhere and isn't keying on language as it should.

I can't dig further in this right now.

I noticed this change in the patch, in that test that is failing if you run more than one of the pieces:

-  protected function assertTranslations($renderer_id, array $expected, $message = '', $group = 'Other') {
-    $view = Views::getView('test_entity_row_renderers');
+  protected function assertTranslations($view, $renderer_id, array $expected, $message = '', $group = 'Other') {
+    $view = clone $view;

The former code is calling Views::getView(), which is documented as "Loads a view from configuration and returns its executable object.". The new code is just doing a clone of an existing object -- and PHP clones (by default, in the absence of a __clone() method on the class) are shallow, so the display and other member variables on the view object would be retained, rather than cloned themselves.. Do the tests pass if you revert that change from the patch?

Makes sense to me, FWIW.

Great work!

1. +++ b/core/modules/views/src/Entity/Render/ConfigurableLanguageRenderer.php
   @@ -0,0 +1,51 @@
   +/**
   + * Renders entities in a configured language. Falls back to current language.
   + */
   

   It would be great to specific what 'current' means in this context (I guess its the content language)

2. +++ b/core/modules/views/src/Plugin/views/PluginBase.php
   @@ -52,6 +52,13 @@
      /**
   +   * Include entity row languages when listing languages.
   +   *
   +   * @see \Drupal\views\Plugin\views\PluginBase::listLanguages()
   +   */
   +  const INCLUDE_ENTITY = 32;
   

   Its just sad that we blow up PluginBase more and more ... can't we move the languages logic into a trait? Work for a follow up?

3. +++ b/core/modules/views/src/Plugin/views/field/Field.php
   @@ -886,16 +894,23 @@ protected function addSelfTokens(&$tokens, $item) {
       */
   -  function field_langcode(EntityInterface $entity) {
   +  public function getFieldLangcode(EntityInterface $entity, ResultRow $row) {
   

   Can we mark it as protected? I know it would be kinda an API change, but at the same time, I think it was never intended to be supported in the first place.

4. +++ b/core/modules/views/tests/src/Unit/Plugin/field/FieldTest.php
   @@ -565,4 +576,41 @@ public function providerSortOrders() {
   +   */
   +  protected function setupLanguageRenderer(Field $handler, $definition) {
   +    $display_handler = $this->getMockBuilder('\Drupal\views\Plugin\views\display\DisplayPluginBase')
   

   Just general thoughts. Sometimes I think making dependencies / features optional in classes would be a good idea to remove complexity.

Seems alright for me.

git ac https://www.drupal.org/files/issues/2394883-entity-language-options-88.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  197k  100  197k    0     0   401k      0 --:--:-- --:--:-- --:--:--  426k
error: patch failed: core/modules/views/src/Plugin/views/field/Field.php:11
error: core/modules/views/src/Plugin/views/field/Field.php: patch does not apply

This is a lovely change. And it's not just the diff stat :) 188 files changed, 470 insertions, 858 deletions.This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed f100d94 and pushed to 8.0.x. Thanks!

Yay!

I created #2420737: Differences in dynamic language names are confusing in views, content, etc. to address the remaining UX issues.

WOOOOOOOOT!

+++ b/core/modules/views/src/Entity/Render/EntityTranslationRenderTrait.php
@@ -0,0 +1,92 @@
+  abstract public function getEntityTypeId();

was there a reason this was public?