Local task link "Translate" is not translated

Yes, that is a bug! Thanks for finding it and also for the patch!

However, using the t() function is discouraged in classes.
We should add a use StringTranslationTrait; to the top of the class and then instead of t() we can do $this->t().

I ran the test localy and I have the same errors. Here is the error message:

/home/maxocub/code/drupal/core/modules/content_translation/tests/src/Unit/Menu/ContentTranslationLocalTasksTest.php:48

2) Drupal\Tests\content_translation\Unit\Menu\ContentTranslationLocalTasksTest::testBlockAdminDisplay with data set #1 ('entity.node.content_translati...erview', array(array('content_translation.local_tas...erview', 'entity.node.canonical', 'entity.node.edit_form', 'entity.node.delete_form', 'entity.node.version_history')))
Symfony\Component\DependencyInjection\Exception\InvalidArgumentException: The service definition "string_translation" does not exist.

I think we need to add a mock for string_translation in the test setup.

Here's an updated patch where I add the string_translation stub to the failing test.
I'm not sure if the translation is actually tested with the stub or if it just pass through.
Any input?

Some screenshots to help with the review:

Nice patch! Thanks.

1. +++ b/core/modules/content_translation/src/Plugin/Derivative/ContentTranslationLocalTasks.php
   @@ -11,11 +11,13 @@
   +use Drupal\Core\StringTranslation\StringTranslationTrait;
   ...
   +  use StringTranslationTrait;
   

   I see this is used to allow for $this->t() so this is good.

2. 
   ...
   diff --git a/core/modules/content_translation/tests/src/Unit/Menu/ContentTranslationLocalTasksTest.php b/core/modules/content_translation/tests/src/Unit/Menu/ContentTranslationLocalTasksTest.php
   ...
   +    $string_translation = $this->getStringTranslationStub();
   +    \Drupal::getContainer()->set('string_translation', $string_translation);
   

   Nitpick: The other examples in the core code (not many) that are setting this are not creating it as separate variable, e.g. in:

   core/modules/node/tests/src/Unit/Plugin/views/field/NodeBulkFormTest.php

   it uses:

   $container->set('string_translation', $this->getStringTranslationStub());

   so I would be inclined to change this to one line like:

   \Drupal::getContainer()->set('string_translation', $this->getStringTranslationStub());

   but this is not a big deal. Since this pattern of using the stub in the container is used in other tests, then I assume this is ok.

I have tested this with simplytest.me and it is working as expected.

Before patch:

After patch:

I'm marking RTBC because my code review only had a minor nitpick that isn't necessary to change. Thanks!

Translate is an existing string so adding yet another place where it is translated is not adding an strings to translate - therefore I think we can fix this in 8.0.x as well.

+++ b/core/modules/content_translation/src/Plugin/Derivative/ContentTranslationLocalTasks.php
@@ -11,11 +11,13 @@
 class ContentTranslationLocalTasks extends DeriverBase implements ContainerDeriverInterface {
+  use StringTranslationTrait;

We should inject the string translation service into the deriver. See \Drupal\field_ui\Plugin\Derivative\FieldUiLocalTask for how this is done.

@Kristen Pol: I corrected the nitpick ;)

@alexpott: Is that what you mean? I'm not sure.

public function __construct($base_plugin_id, ContentTranslationManagerInterface $content_translation_manager, TranslationInterface $string_translation) {
  $this->basePluginId = $base_plugin_id;
  $this->contentTranslationManager = $content_translation_manager;
  $this->stringTranslation = $string_translation;
}

public static function create(ContainerInterface $container, $base_plugin_id) {
  return new static(
     $base_plugin_id,
     $container->get('content_translation.manager'),
     $container->get('string_translation')
  );
}

+++ b/core/modules/content_translation/src/Plugin/Derivative/ContentTranslationLocalTasks.php
@@ -11,11 +11,14 @@
+use Drupal\Core\StringTranslation\StringTranslationTrait;

@@ -38,10 +41,13 @@ class ContentTranslationLocalTasks extends DeriverBase implements ContainerDeriv
+  public function __construct($base_plugin_id, ContentTranslationManagerInterface $content_translation_manager, TranslationInterface $string_translation) {
     $this->basePluginId = $base_plugin_id;
     $this->contentTranslationManager = $content_translation_manager;

@@ -50,7 +56,8 @@ public function __construct($base_plugin_id, ContentTranslationManagerInterface
-      $container->get('content_translation.manager')
+      $container->get('content_translation.manager'),
+      $container->get('string_translation')
     );

You no longer have to inject it, because $this->t() just creates an object

One second - @dawehner and I are going to discuss this.

@dawehner and I discussed this in IRC.

11:33 alexpott
dawehner: I'm not sure I agree
11:34 dawehner
alexpott: why?
11:34 alexpott
dawehner: the way it is on the patch the TranslatableMarkup object will contain a reference to the string.translation service
11:34 dawehner
alexpott: I would agree on low level services
11:34 alexpott
dawehner: if it doesn't - it will just using the service location anti pattern to get it
11:34 dawehner
but this is just yet another plugin like code
11:35 dawehner
alexpott: well, ideally the renderer code would inject it in there
11:35 miro_dietiker has left IRC (Client Quit)
11:35 navneet0693 has left IRC (Remote host closed the connection)
11:36 alexpott
dawehner: you are right... I'm not fussed either way tbh - it'd be nice to work out when to inject string.translation and when to not
11:36 alexpott
dawehner: atm we're inconsistent
11:36 dawehner
alexpott: I agree, and this is annoying
11:36 alexpott
dawehner: yes
11:36 dawehner
alexpott: my rule of thumb would be: services: inject it, otherwise don't
11:36 alexpott
dawehner: which is why I tend for inject it where possible since that is an easy rule
11:37 dawehner
I agree
11:37 dawehner
so yeah we could just say: do it, its better, but not doing it is not the end of the world

so tldr; the latest patch is fine and we have work to do on when and when not to inject this.

Per #24, "so tldr; the latest patch is fine and we have work to do on when and when not to inject this." so moving this back to RTBC since that's what I interpret "the latest patch is fine" to mean.

Committed 366380d and pushed to 8.0.x, 8.1.x and 8.2.x. Thanks!

Yay, thanks!