Replace the hardcoded langcode key on blocks with the 'language' cache context

No test coverage yet, but this should fix the problem: we make sure that this block is always cached per language.

Turns out that using the language entity is not an option, because it's only available when the language module is enabled. Which would mean we can only add cache tags when the language module is enabled, which is less than great.

So I added a getCacheTag() method to LanguageInterface.

Perhaps this issue should be broadened in scope; it could become the "cache tags for languages" issue.

I wonder whether we could just add language somehow

+++ b/core/modules/block/src/BlockBase.php
@@ -447,7 +447,10 @@ public function getCacheTags() {
+    return array_merge(
+      array('block_plugin' => array($block_plugin_cache_tag)),
+      \Drupal::languageManager()->getCurrentLanguage()->getCacheTag()
+    );

You could argue that we could put this even higher in the chain. Could the cache.render cache service always include it?

#9: I've thought about putting it higher in the chain too. It couldn't live in the cache.render cache back-end though, but it could live in cache_render_cache_set().
But that would make it almost impossible to choose not to render cache per-language. It's really the responsibility of each renderable thing to declare by what it needs to be invalidated, by setting the proper cache tags.

BTW I think it would be good to start to practically introduce language cache tags here, and then spread them instead of starting even one more level up...

Agreed, so I'm starting to do that now.

I updated the patch to vary each block per language (using the "language" cache context), because if we're going to tag every block with the current language's cache tag, we should also vary each block per language. This was already done "kind of": each block got the LanguageManager->getCurrentLanguage()->getId() key, but we might just as well use the cache context, because otherwise we already cache blocks per UI language, but still allow users to configure blocks to be cached per language. That's rather confusing.
This caused one subtle change: blocks no longer get the en part of their cache ID while there is only a single language on the website, they'll get the empty string instead. I think it's clearer for everybody if we always include the language explicitly, so I'm updating LanguageCacheContext slightly.

We'll also want to bring the language cache tag to entities (nodes etc.), because they may be showing field labels. And by default there's the Submitted by %author% on %locale-formatted-date% text, which also depends on the UI language. We may also want to introduce cache_context.language there, but EntityViewBuilder::getBuildDefaults() currently has

      if ($entity instanceof TranslatableInterface && count($entity->getTranslationLanguages()) > 1) {
        $build['#cache']['keys'][] = $langcode;
      }

So I'd love input from e.g. Gábor on how to correctly handle all that.

We generally should be careful about UI language, content language, and available translations of entities when doing adding cache tags and varying by language.

The STR in the IS are fixed with this patch. Still needed: a test that follows the STR.

(Patch now simplified because of #12/#14. No interdiff because the patch is now tiny, thanks to its focus. A new issue should be filed to deal with the cache tags aspect that was handled in #11.)

Forgot to include one hunk in #19.

Let's see if somebody wants to write this test :)

I'll write a test for this.

Wonderful, thank you!

I'm actually not able to reproduce this following the STR in the IS.
Not sure if I'm overlooking something, can someone confirm that the STR in the IS are still relevant?

What is STR and IS?

@rodrigoaguilera
STR: Steps to reproduce
IS: Issue summary

Confirmed, I also can't reproduce this anymore.

It feels like something is missing from the STR. Hopefully Gábor can enlighten us :)

But blocks have always been cached by language already, just in a hardcoded, less-than-great way:

+++ b/core/modules/block/src/BlockViewBuilder.php
@@ -84,7 +84,8 @@ public function viewMultiple(array $entities = array(), $view_mode = 'full', $la
-          $this->languageManager->getCurrentLanguage()->getId(),
+          // Blocks are always rendered in a "per language" cache context.
+          'cache_context.language',

I think this is what happened originally:

1. Custom block "Hello" created, in English.
2. All the translation stuff is enabled. Let's say we added French as a site language.
3. Visit a page with the "Hello" block, in French. But there's no translation. So we see "Hello". So far so good.
4. Only *now* we translate the block to "Bonjour". When going to the same page as the previous step, we still see "Hello".

What has changed since the above, is simple: config cache tags. We now consistently invalidate cache tags for config: #2040135: Caches dependent on simple config are only invalidated on form submissions — even when not using the config entity, but config itself directly. AFAICT, config_translation uses config "directly" (not config entities). So those CRUD operations now trigger the cache tag invalidations for the placed block config entity, whose cache tags live on the page, and voila: it all works!

If Gábor can confirm the above explanation, I think we should repurpose this issue to not be a bugfix, but a task, to still do what #20 does, because it's still a valid cleanup.

I also think #20 is still a valid change, I can still write a test to confirm that the cache tag is actually invalidated on form submission what will still happen when I apply this issue if I'm not wrong.

I don't think a test is necessary. This is now basically a code clean-up. Retitled.

This fixes that.

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 0d869b1 and pushed to 8.0.x. Thanks!