Not all node, taxonomy entity tokens are multilingual aware

Related issue need review for a long time

no other issues, but it makes sense to have common test class for them all

This is what I have done so far. Tests for translated tokens are still missing.

Wanted to see if this is a starting point for tests.

Working on extending these tests.

@legolasbo Hi. I finished some part of the test. If you didn't do much work maybe its ok of I take over this part?

@Denchev,

I've changed your patch to reduce code duplication and actually test for correct token replacements. Feel free to take over from here. It still needs to test for the token replacement of the other fields.

After uploading this patch I'll do a code review of the patch from #4.

Yes, that last patch is definitely an improvement.

This is about testing tokens, not the node UI. We should be able to limit it to only use the API, just create a node with Node::create(); save, and then ->addTranslation('it') or so, save that too. Then generate tokens.

Patch didn't apply. Performed a basic reroll and removed some scope creep caused by array reformatting while fixing merge conflicts.

Fixing failing test on #11

The tests will probably pass but its not correct yet.

Need some feedback on translating the author, and for some reason the "changed" and "created" using the service date.formater and passing the langcode didn't translate the string correctly its still in English.

1. +++ b/core/modules/node/src/Tests/NodeTokenLanguageTest.php
   @@ -63,73 +70,112 @@ protected function setUp() {
   +    $this->assertEqual($node->getOwner()->getUsername(), User::load($fields['uid'])->getUsername(), 'Owner username matches ');
   

   When we created the node, we set the user id to $this->user->id(). Can we use here $this->user->getUsername() instead of User::load($fields['uid'])->getUsername()?

2. +++ b/core/modules/node/src/Tests/NodeTokenLanguageTest.php
   @@ -63,73 +70,112 @@ protected function setUp() {
   +    $token = '[node:title]';
   +    $token_replacement = \Drupal::token()->replace($token, ['node' => $node], $options);
   +    $this->assertEqual($token_replacement, $node->title->value, $token . ' replaced successfully');
   ...
   +    $this->assertEqual($token_replacement, $node->body->value, $token . ' replaced successfully');
   ...
   +    $this->assertEqual($token_replacement, $node->getOwner()->getUsername(), $token . ' replaced successfully');
   ...
   +    $this->assertEqual($token_replacement, \Drupal::service('date.formatter')->format($node->created->value, 'medium', '', NULL, $langcode), $token . ' replaced successfully');
   ...
   +    $this->assertEqual($token_replacement, \Drupal::service('date.formatter')->format($node->changed->value, 'medium', '', NULL, $langcode), $token . ' replaced successfully');
   ...
   -  protected function verifyTokensCorrectlyReplaced($node, $fields, $langcode) {
   -    foreach ($fields as $field_name => $field_value) {
   -      $token = '[node:' . $field_name . ']';
   -      // Perform unsanitized replacement for easy comparison.
   -      $options = [
   -        'langcode' => $langcode,
   -        'sanitize' => FALSE,
   -      ];
   -      $token_replacement = \Drupal::token()
   -        ->replace($token, ['node' => $node], $options);
   -      $this->assertEqual($token_replacement, $field_value, $token . ' replaced successfully');
   -    }
   -  }
   

   Can we just modify verifyTokensCorrectlyReplaced() method, pass the required values (e.g. mapping of token names - field values) and call it once as a helper method instead of calling assertEquals() multiple times? I guess it will be more readable.

working on this.

This patch addresses 16.2

Author translation is now properly tested. Also had a look at the created/changed fields. They are being translated properly, but the italian translations don't seem to be imported when enabling a language in the test. The date string is therefor still in English, because it falls back to English. The behaviour of the translation api is however thoroughly tested, so I think we are fine like this.

I removed the following because we don't need to test the node/translation creation. We only want to test the token replacement.

-    // Check that the node translation exists in the database.
-    $this->assertEqual($translation->language()->getId(), $langcode, 'Node has langcode: ' . $langcode);
-    $this->assertEqual($translation->title->value, $fields['title'], 'Title value matches.');
-    $this->assertEqual($translation->body->value, $fields['body'], 'Body value matches.');
-    $this->assertEqual($translation->getOwner()->getUsername(), User::load($fields['uid'])->getUsername(), 'Owner username matches ');
-
-    $this->assertEqual($translation->created->value, $fields['created'], 'Created value matches');
-    $this->assertEqual($translation->changed->value, $fields['changed'], 'Changed value matches');

Added term tests

1. +++ b/core/modules/node/node.tokens.inc
   @@ -86,6 +85,11 @@ function node_token_info() {
   +  if ($type != 'node' || empty($data['node'])) {
   +    return [];
   +  }
   
   @@ -94,119 +98,110 @@ function node_tokens($type, $tokens, array $data, array $options, BubbleableMeta
   -  if ($type == 'node' && !empty($data['node'])) {
   

   Making this change makes the patch way harder to review.

2. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -78,75 +96,106 @@ function testNodeTokenReplacement() {
   +  protected function replaceTokensAndCheckMetadata(array $tests, array $data, array $options, $message, array $metadata_tests) {
   ...
   +  protected function replaceTokens(array $tests, array $data, array $options, $message) {
   
   +++ b/core/modules/taxonomy/src/Tests/TokenReplaceTest.php
   @@ -156,18 +182,42 @@ function testTaxonomyTokenReplacement() {
   +  protected function replaceTokensAndCheckMetadata(array $tests, array $data, array $options, $message, array $metadata_tests) {
   ...
   +  protected function replaceTokens(array $tests, array $data, array $options, $message) {
   

   Missing docs

3. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -78,75 +96,106 @@ function testNodeTokenReplacement() {
   +      $this->assertEqual($output, $expected, format_string($message, [
   ...
   +      $this->assertEqual($output, $expected, format_string($message, [
   
   +++ b/core/modules/taxonomy/src/Tests/TokenReplaceTest.php
   @@ -156,18 +182,42 @@ function testTaxonomyTokenReplacement() {
   +      $this->assertEqual($output, $expected, format_string($message, [
   ...
   +      $this->assertEqual($output, $expected, format_string($message, [
   

   Use SafeMarkup::format() - format_string() is deprecated.

4. +++ b/core/modules/taxonomy/taxonomy.tokens.inc
   @@ -117,28 +136,30 @@ function taxonomy_tokens($type, $tokens, array $data, array $options, Bubbleable
            case 'node-count':
   -          $query = db_select('taxonomy_index');
   -          $query->condition('tid', $term->id());
   -          $query->addTag('term_node_count');
   -          $count = $query->countQuery()->execute()->fetchField();
   -          $replacements[$original] = $count;
   +          $replacements[$original] = db_select('taxonomy_index')
   +            ->condition('tid', $term->id())
   +            ->addTag('term_node_count')
   +            ->countQuery()
   +            ->execute()
   +            ->fetchField();
              break;
   

   Afaics this has not changed a thing.

On this!

22.1 Reverted that out of scope change and also some additional out of scope code style changes. Greatly simplified the patch for review.
22.2 Documentation added
22.3 done
22.4 Did not change a thing indeed. Reverted the change.

Oops. forgot to restore $replacements variable.

I forgot Simpletest tries to run every function which starts with test. Should pass now.

1. +++ b/TaxonomyTokenLanguageTest.php
   @@ -0,0 +1,115 @@
   +    // Configure the vocabulary to not hide the language selector.
   +    // TODO do we need this?
   

   I see no need in that

2. +++ b/TaxonomyTokenLanguageTest.php
   @@ -0,0 +1,115 @@
   +  }
   +}
   

   needs empty line between

3. +++ b/core/modules/node/src/Tests/NodeTokenLanguageTest.php
   @@ -0,0 +1,158 @@
   +  /**
   +   * @var null
   +   */
   +  private $users = NULL;
   

   needs description

4. +++ b/core/modules/node/src/Tests/NodeTokenLanguageTest.php
   @@ -0,0 +1,158 @@
   +
   +  protected function setUp() {
   

   needs {@inheritdoc}

5. +++ b/core/modules/node/src/Tests/NodeTokenLanguageTest.php
   @@ -0,0 +1,158 @@
   +   * @param $langcode
   +   *
   +   * @return array
   

   string $langcode
   and both needs description

6. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -24,38 +26,52 @@ class NodeTokenReplaceTest extends TokenReplaceUnitTestBase {
   +  /**
   +   * @var NodeTypeInterface
   +   */
   +  protected $nodeType;
   

   should be full namespace and needs description

7. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -24,38 +26,52 @@ class NodeTokenReplaceTest extends TokenReplaceUnitTestBase {
   +    $this->nodeType = entity_create('node_type', [
   

   NodeType::create([..])

8. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -90,63 +107,125 @@ function testNodeTokenReplacement() {
   +  /**
   +   * Asserts if tokens are correctly replaced and verifies that the correct
   +   * metadata has been set.
   
   +++ b/core/modules/taxonomy/src/Tests/TokenReplaceTest.php
   @@ -156,18 +182,75 @@ function testTaxonomyTokenReplacement() {
   +   * Asserts if tokens are correctly replaced and verifies that the correct
   +   * metadata has been set.
   

   should be one-liner

9. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -90,63 +107,125 @@ function testNodeTokenReplacement() {
   +   * @param array $tests
   +   * Expected results keyed by their respective tokens.
   +   * @param array $data
   +   * The data to perform the replacement on. @see Token::replace().
   +   * @param array $options
   +   * Aditional replacement options. @see Token::replace().
   +   * @param $message
   +   * The message to display with the assertion. Can contain replacement tokens:
   +   * - %token for the token name
   +   * - %output for the value returned by the token replacement service
   +   * - %expected for the expected result
   +   * @param array $metadata_tests
   +   * The metadata to verify. Keyed by the relevant token.
   ...
   +   * @param array $tests
   +   * Expected results keyed by their respective tokens.
   +   * @param array $data
   +   * The data to perform the replacement on. @see Token::replace().
   +   * @param array $options
   +   * Aditional replacement options. @see Token::replace().
   +   * @param $message
   +   * The message to display with the assertion. Can contain replacement tokens:
   +   * - %token for the token name
   +   * - %output for the value returned by the token replacement service
   +   * - %expected for the expected result
   
   +++ b/core/modules/taxonomy/src/Tests/TokenReplaceTest.php
   @@ -156,18 +182,75 @@ function testTaxonomyTokenReplacement() {
   +   * @param array $tests
   +   * Expected results keyed by their respective tokens.
   +   * @param array $data
   +   * The data to perform the replacement on. @see Token::replace().
   +   * @param array $options
   +   * Aditional replacement options. @see Token::replace().
   +   * @param $message
   +   * The message to display with the assertion. Can contain replacement tokens:
   +   * - %token for the token name
   +   * - %output for the value returned by the token replacement service
   +   * - %expected for the expected result
   +   * @param array $metadata_tests
   +   * The metadata to verify. Keyed by the relevant token.
   ...
   +   * @param array $tests
   +   * Expected results keyed by their respective tokens.
   +   * @param array $data
   +   * The data to perform the replacement on. @see Token::replace().
   +   * @param array $options
   +   * Aditional replacement options. @see Token::replace().
   +   * @param $message
   +   * The message to display with the assertion. Can contain replacement tokens:
   +   * - %token for the token name
   +   * - %output for the value returned by the token replacement service
   +   * - %expected for the expected result
   

   needs proper indent, looks like copy-paste

Should address all from #31.

1. +++ b/core/modules/node/src/Tests/NodeTokenLanguageTest.php
   @@ -27,10 +27,15 @@ class NodeTokenLanguageTest extends WebTestBase {
   +   * Users for testing different language.
   

   s/language/languages

2. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -12,10 +12,10 @@
   + * Generates text using placeholders for dummy content to check node token replacement.
   

   Exceeds 80 characters.

3. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -147,47 +151,46 @@ function testNodeTokenReplacement() {
   +   * Asserts if tokens are correctly replaced and verifies that the correct metadata has been set.
   

   Exceeds 80 characters

4. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -147,47 +151,46 @@ function testNodeTokenReplacement() {
   +   *   The message to display with the assertion. Can contain replacement tokens:
   

   Exceeds 80 characters

5. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -207,16 +210,16 @@ protected function assertTokenReplacementAndCheckMetadata(array $tests, array $d
   +   *   The message to display with the assertion. Can contain replacement tokens:
   

   Exceeds 80 characters

+++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
@@ -24,129 +26,209 @@ class NodeTokenReplaceTest extends TokenReplaceUnitTestBase {
-      'body' => array(array('value' => $this->randomMachineName(32), 'summary' => $this->randomMachineName(16), 'format' => 'plain_text')),
-    ));
-    $node->save();
-
-    // Generate and test sanitized tokens.
-    $tests = array();
-    $tests['[node:nid]'] = $node->id();
-    $tests['[node:vid]'] = $node->getRevisionId();
-    $tests['[node:type]'] = 'article';
-    $tests['[node:type-name]'] = 'Article';
-    $tests['[node:title]'] = Html::escape($node->getTitle());
-    $tests['[node:body]'] = $node->body->processed;
-    $tests['[node:summary]'] = $node->body->summary_processed;
-    $tests['[node:langcode]'] = Html::escape($node->language()->getId());
-    $tests['[node:url]'] = $node->url('canonical', $url_options);
-    $tests['[node:edit-url]'] = $node->url('edit-form', $url_options);
-    $tests['[node:author]'] = Html::escape($account->getUsername());
-    $tests['[node:author:uid]'] = $node->getOwnerId();
-    $tests['[node:author:name]'] = Html::escape($account->getUsername());
-    $tests['[node:created:since]'] = \Drupal::service('date.formatter')->formatTimeDiffSince($node->getCreatedTime(), array('langcode' => $this->interfaceLanguage->getId()));
-    $tests['[node:changed:since]'] = \Drupal::service('date.formatter')->formatTimeDiffSince($node->getChangedTime(), array('langcode' => $this->interfaceLanguage->getId()));
-
-    $base_bubbleable_metadata = BubbleableMetadata::createFromObject($node);
-
-    $metadata_tests = [];
-    $metadata_tests['[node:nid]'] = $base_bubbleable_metadata;
-    $metadata_tests['[node:vid]'] = $base_bubbleable_metadata;
-    $metadata_tests['[node:type]'] = $base_bubbleable_metadata;
-    $metadata_tests['[node:type-name]'] = $base_bubbleable_metadata;
-    $metadata_tests['[node:title]'] = $base_bubbleable_metadata;
-    $metadata_tests['[node:body]'] = $base_bubbleable_metadata;
-    $metadata_tests['[node:summary]'] = $base_bubbleable_metadata;
-    $metadata_tests['[node:langcode]'] = $base_bubbleable_metadata;
-    $metadata_tests['[node:url]'] = $base_bubbleable_metadata;
-    $metadata_tests['[node:edit-url]'] = $base_bubbleable_metadata;
-    $bubbleable_metadata = clone $base_bubbleable_metadata;
-    $metadata_tests['[node:author]'] = $bubbleable_metadata->addCacheTags(['user:1']);
-    $metadata_tests['[node:author:uid]'] = $bubbleable_metadata;
-    $metadata_tests['[node:author:name]'] = $bubbleable_metadata;
-    $bubbleable_metadata = clone $base_bubbleable_metadata;
-    $metadata_tests['[node:created:since]'] = $bubbleable_metadata->setCacheMaxAge(0);
-    $metadata_tests['[node:changed:since]'] = $bubbleable_metadata;
-
-    // Test to make sure that we generated something for each token.
-    $this->assertFalse(in_array(0, array_map('strlen', $tests)), 'No empty tokens generated.');
-
-    foreach ($tests as $input => $expected) {
-      $bubbleable_metadata = new BubbleableMetadata();
-      $output = $this->tokenService->replace($input, array('node' => $node), array('langcode' => $this->interfaceLanguage->getId()), $bubbleable_metadata);
-      $this->assertEqual($output, $expected, format_string('Sanitized node token %token replaced.', array('%token' => $input)));
-      $this->assertEqual($bubbleable_metadata, $metadata_tests[$input]);
-    }
-
-    // Generate and test unsanitized tokens.
-    $tests['[node:title]'] = $node->getTitle();
-    $tests['[node:body]'] = $node->body->value;
-    $tests['[node:summary]'] = $node->body->summary;
-    $tests['[node:langcode]'] = $node->language()->getId();
-    $tests['[node:author:name]'] = $account->getUsername();
-
-    foreach ($tests as $input => $expected) {
-      $output = $this->tokenService->replace($input, array('node' => $node), array('langcode' => $this->interfaceLanguage->getId(), 'sanitize' => FALSE));
-      $this->assertEqual($output, $expected, format_string('Unsanitized node token %token replaced.', array('%token' => $input)));
-    }
-
-    // Repeat for a node without a summary.
-    $node = entity_create('node', array(
-      'type' => 'article',
+      'body' => [
+        [
+          'value' => '<blink>Blinking Body</blink>',
+          'summary' => '<blink>Blinking Summary</blink>',
+          'format' => 'plain_text',
+        ],
+      ],
+      ));
+      $node->save();
+
+      // Generate and test sanitized tokens.
+      $tests = array();
+      $tests['[node:nid]'] = $node->id();
+      $tests['[node:vid]'] = $node->getRevisionId();
+      $tests['[node:type]'] = $this->nodeType->id();
+      $tests['[node:type-name]'] = Html::escape($this->nodeType->label());
+      $tests['[node:title]'] = Html::escape($node->getTitle());
+      $tests['[node:body]'] = $node->body->processed;
+      $tests['[node:summary]'] = $node->body->summary_processed;
+      $tests['[node:langcode]'] = Html::escape($node->language()->getId());
+      $tests['[node:url]'] = $node->url('canonical', $url_options);
+      $tests['[node:edit-url]'] = $node->url('edit-form', $url_options);
+      $tests['[node:author]'] = Html::escape($account->getUsername());
+      $tests['[node:author:uid]'] = $node->getOwnerId();
+      $tests['[node:author:name]'] = Html::escape($account->getUsername());
+      $tests['[node:created:since]'] = \Drupal::service('date.formatter')
+        ->formatTimeDiffSince($node->getCreatedTime(), array('langcode' => $this->interfaceLanguage->getId()));
+      $tests['[node:changed:since]'] = \Drupal::service('date.formatter')
+        ->formatTimeDiffSince($node->getChangedTime(), array('langcode' => $this->interfaceLanguage->getId()));
+
+      $base_bubbleable_metadata = BubbleableMetadata::createFromObject($node);
+      $author_bubbleable_metadata = clone $base_bubbleable_metadata;
+      $date_bubbleable_metadata = clone $base_bubbleable_metadata;
+      $metadata_tests = [];
+      $metadata_tests['[node:nid]'] = $base_bubbleable_metadata;
+      $metadata_tests['[node:vid]'] = $base_bubbleable_metadata;
+      $metadata_tests['[node:type]'] = $base_bubbleable_metadata;
+      $metadata_tests['[node:type-name]'] = $base_bubbleable_metadata;
+      $metadata_tests['[node:title]'] = $base_bubbleable_metadata;
+      $metadata_tests['[node:body]'] = $base_bubbleable_metadata;
+      $metadata_tests['[node:summary]'] = $base_bubbleable_metadata;
+      $metadata_tests['[node:langcode]'] = $base_bubbleable_metadata;
+      $metadata_tests['[node:url]'] = $base_bubbleable_metadata;
+      $metadata_tests['[node:edit-url]'] = $base_bubbleable_metadata;
+      $metadata_tests['[node:author]'] = $author_bubbleable_metadata->addCacheTags(['user:1']);
+      $metadata_tests['[node:author:uid]'] = $author_bubbleable_metadata;
+      $metadata_tests['[node:author:name]'] = $author_bubbleable_metadata;
+      $metadata_tests['[node:created:since]'] = $date_bubbleable_metadata->setCacheMaxAge(0);
+      $metadata_tests['[node:changed:since]'] = $date_bubbleable_metadata;
+
+      // Test to make sure that we generated something for each token.
+      $this->assertFalse(in_array(0, array_map('strlen', $tests)), 'No empty tokens generated.');
+
+      $data = ['node' => $node];
+      $options = [
+      'langcode' => $this->interfaceLanguage->getId(),
+      'sanitize' => TRUE,
+      ];
+      $msg = 'Sanitized node token %token replaced with %output. Expected is %expected';
+      $this->assertTokenReplacementAndCheckMetadata($tests, $data, $options, $msg, $metadata_tests);
+
+      // Generate and test unsanitized tokens.
+      $tests['[node:type-name]'] = $this->nodeType->label();
+      $tests['[node:title]'] = $node->getTitle();
+      $tests['[node:body]'] = $node->body->value;
+      $tests['[node:summary]'] = $node->body->summary;
+      $tests['[node:langcode]'] = $node->language()->getId();
+      $tests['[node:author]'] = $account->getUsername();
+      $tests['[node:author:name]'] = $account->getUsername();
+      $options['sanitize'] = FALSE;
+      $msg = 'Unsanitized node token %token replaced with %output. Expected is %expected';
+      $this->assertTokenReplacement($tests, $data, $options, $msg);
+
+      // Repeat for a node without a summary.
+      $node = entity_create('node', array(
+      'type' => $this->nodeType->id(),
...
-      'body' => array(array('value' => $this->randomMachineName(32), 'format' => 'plain_text')),
-    ));
-    $node->save();
-
-    // Generate and test sanitized token - use full body as expected value.
-    $tests = array();
-    $tests['[node:summary]'] = $node->body->processed;
-
-    // Test to make sure that we generated something for each token.
-    $this->assertFalse(in_array(0, array_map('strlen', $tests)), 'No empty tokens generated for node without a summary.');
+      'body' => [
+        [
+          'value' => '<blink>Blinking Body without Summary</blink>',
+          'format' => 'plain_text',
+        ],
+      ],
+      ));
+      $node->save();
+
+      // Generate and test sanitized token - use full body as expected value.
+      $tests = [
+        '[node:summary]' => $node->body->processed,
+      ];
+
+      // Test to make sure that we generated something for each token.
+      $this->assertFalse(in_array(0, array_map('strlen', $tests)), 'No empty tokens generated for node without a summary.');
+
+      $data = ['node' => $node];
+      $options['sanitize'] = TRUE;
+      $msg = 'Sanitized node token %token replaced with %output for node without a summary. Expected is %expected';
+      $this->assertTokenReplacement($tests, $data, $options, $msg);
+
+      // Generate and test unsanitized tokens.
+      $tests['[node:summary]'] = $node->body->value;
+
+      $options['sanitize'] = FALSE;
+      $msg = 'Unsanitized node token %token replaced with %output for node without a summary. Expected is %expected';
+      $this->assertTokenReplacement($tests, $data, $options, $msg);

This codestyle change makes the patch much harder to review and should be reverted.

Addressed 1-5 and some styling errors that i noticed. Will maybe take a look later at the big one.

#33.6 needs to be addressed.

#33.6 Reverted changes, hope i understood that right

@Denchev,

Patch no longer applies and needs a reroll, also found more unneeded codestyle changes.

1. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -55,7 +57,13 @@ function testNodeTokenReplacement() {
   -      'body' => array(array('value' => $this->randomMachineName(32), 'summary' => $this->randomMachineName(16), 'format' => 'plain_text')),
   +      'body' => array(
   +        array(
   +          'value' => $this->randomMachineName(32),
   +          'summary' => $this->randomMachineName(16),
   +          'format' => 'plain_text',
   +        ),
   +      ),
   

   Codestyle changes, add review noise.

2. +++ b/core/modules/taxonomy/src/Tests/TaxonomyTestTrait.php
   @@ -18,10 +18,16 @@
   +  function createVocabulary(array $values = array()) {
   ...
   +    $vocabulary = entity_create('taxonomy_vocabulary', $values + array(
   

   Use shorthand array notation for all newly introduced/updated arrays

3. +++ b/core/modules/taxonomy/src/Tests/TokenReplaceTest.php
   @@ -34,10 +37,25 @@ class TokenReplaceTest extends TaxonomyTestBase {
   -    $this->vocabulary = $this->createVocabulary();
   +    $this->vocabulary = $this->createVocabulary([
   +      'name' => 'V1 <strong>"&lt;name&gt;"</strong>',
   +    ]);
   

   let's put this on a single line

4. +++ b/core/modules/taxonomy/src/Tests/TokenReplaceTest.php
   @@ -63,24 +81,29 @@ protected function setUp() {
   -    $edit = array();
   -    $node = $this->drupalCreateNode(array('type' => 'article'));
   -    $edit[$this->fieldName . '[]'] = $term2->id();
   +    $node = $this->drupalCreateNode(['type' => 'article']);
   +    $edit = [
   +      $this->fieldName . '[]' => $term2->id(),
   +    ];
   

   Codestyle changes add review noise.

5. +++ b/core/modules/taxonomy/src/Tests/TokenReplaceTest.php
   @@ -90,29 +113,34 @@ function testTaxonomyTokenReplacement() {
   -    $bubbleable_metadata = clone $base_bubbleable_metadata;
   -    $metadata_tests['[term:vocabulary:name]'] = $bubbleable_metadata->addCacheTags($this->vocabulary->getCacheTags());
   ...
        $metadata_tests['[term:vocabulary]'] = $bubbleable_metadata->addCacheTags($this->vocabulary->getCacheTags());
   +    $metadata_tests['[term:vocabulary:name]'] = $bubbleable_metadata->addCacheTags($this->vocabulary->getCacheTags());
   

   Codestyle changes add review noise. And also seems to have introduced an error since $bubbleable_metadata is now nolonger set.

Lets try to make the patch apply. I will continue to fix whats breaks.

Reroll of #34

I was not really sure about what patch should I reroll.

So I went over the changes from the start and there might be some changes missing, not sure. Here I also tried fixing NodeTokenLanguageTest and NodeTokenLanguageTest by adding some changes to tokens.php. Was wondering if thats ok

hook_tokens() is not responsible for sanitization any more (see https://www.drupal.org/node/2578365 for more info). This effectively means that we don't have $options['sanitize'] any more and we have to remove it from patch entirely.

Removing $options['sanitize'] from #45 patch.

1. +++ b/core/modules/node/node.tokens.inc
   @@ -93,13 +93,15 @@ function node_tokens($type, $tokens, array $data, array $options, BubbleableMeta
      }
   -  $replacements = array();
   +  $replacements = [];
   

   Try to remove as many unrelated changes as possible.

2. +++ b/core/modules/node/node.tokens.inc
   @@ -113,22 +115,21 @@ function node_tokens($type, $tokens, array $data, array $options, BubbleableMeta
            case 'type':
   -          $replacements[$original] = $node->getType();
   +          $replacements[$original] = Html::escape($node->getType());
   

   All those changes are unrelated. You need to drop them, they're incorrectly merged from an updated patch.

On this for a while

I've removed a whole bunch of unrelated changes and all sanitize = true/false i could find. Several tests are still failing but I am out of time. Let's see how testbot thinks about it anyway.

IMHO "needs reroll" tag is not needed

Working on this some more.

Fixed the broken tests and removed some more unrelated code. Also found that the tests don't provide full test coverage where it concerns bubbleable metadata. Will improve test coverage some more.

Let's have testbot take a look to see if I missed something.

This should do it.

I introduced AssertTokenReplacementTrait to reduce code duplication and ensure all token replacements are tested in the same way and also extended the cacheability metadata testing in TokenReplaceTest to cover all tested tokens.

1. +++ b/core/lib/Drupal/Core/Utility/Token.php
   @@ -206,7 +206,7 @@ public function replace($text, array $data = array(), array $options = array(),
   -      $replacements[$token] = $value instanceof MarkupInterface ? $value : new HtmlEscapedText($value);
   +      $replacements[$token] = ($value instanceof MarkupInterface) ? $value : new HtmlEscapedText($value);
        }
   

   This seems like an unrelated change.

2. +++ b/core/modules/node/src/Tests/NodeTokenLanguageTest.php
   @@ -0,0 +1,149 @@
   +  /**
   +   * Users for testing different languages.
   +   *
   +   * @var null
   +   */
   +  private $users = NULL;
   +
   

   This should be typehinted as \Drupal\user\UserInterface[] not null.

   Would also make sense to make it protected instead of private.

3. +++ b/core/modules/node/src/Tests/NodeTokenLanguageTest.php
   @@ -0,0 +1,149 @@
   +    foreach (['en', 'it'] as $langcode) {
   +      $this->users[$langcode] = $this->drupalCreateUser([
   +        'create page content',
   +        'edit own page content'
   +      ]);
   +    }
   

   Is it necessary to have two identical users?

Thanks for the review.

61.1 - Could be unrelated, I don't know why it was added, so I guess we could try reverting the change.
61.2 - You are correct, should be changed.
61.3 - Yes it is necessary to have two "identical" users, because they will differ by username which is what we need to make sure they are properly replaced in different languages.

Unfortunately I don't have time to work on this until Friday at least.

@slashrsm Thanks for feedback

Addressed 61.1 and 61.2

Sorry. Missed the gitignore. Here is a new fixed patch

Looks good to me.

Would be great to get this in and re-use the same trait for #1885962: Comment tokens should use entity translation API

Test failures seem unrelated. Back to RTBC while Retesting.

removed outdated

1. +++ b/core/modules/node/node.tokens.inc
   @@ -6,9 +6,9 @@
   +use Drupal\Component\Utility\Html;
   

   This does not appear to be used.

2. +++ b/core/modules/node/src/Tests/NodeTokenReplaceTest.php
   @@ -13,6 +13,8 @@
   + * Generates text using placeholders.
   + *
   
   @@ -42,7 +44,7 @@ protected function setUp() {
   -  function testNodeTokenReplacement() {
   +  public function testNodeTokenReplacement() {
   

   Out of scope changes.

3. +++ b/core/modules/taxonomy/src/Tests/TaxonomyTestTrait.php
   @@ -18,10 +18,16 @@
   +   * @return \Drupal\taxonomy\VocabularyInterface
   +   *   The new taxonomy vocabulary object.
   
   +++ b/core/modules/taxonomy/src/Tests/TokenReplaceTest.php
   @@ -18,10 +21,12 @@
   -   * @var \Drupal\taxonomy\VocabularyInterface
   +   * @var \Drupal\taxonomy\entity\Vocabulary
       */
      protected $vocabulary;
   
   @@ -32,10 +37,21 @@ class TokenReplaceTest extends TaxonomyTestBase {
   -    $this->vocabulary = $this->createVocabulary();
   +    $this->vocabulary = $this->createVocabulary(['name' => 'V1 <strong>"&lt;name&gt;"</strong>']);
   

   Why are we making the change to the protected property type doc? The method says it is returning the VocabularyInterface

4. +++ b/core/modules/taxonomy/taxonomy.tokens.inc
   @@ -148,14 +152,14 @@ function taxonomy_tokens($type, $tokens, array $data, array $options, Bubbleable
   -    if (($vocabulary_tokens = $token_service->findWithPrefix($tokens, 'parent')) && $parents = $taxonomy_storage->loadParents($term->id())) {
   +    if (($parent_tokens = $token_service->findWithPrefix($tokens, 'parent')) && $parents = $taxonomy_storage->loadParents($term->id())) {
          $parent = array_pop($parents);
   -      $replacements += $token_service->generate('term', $vocabulary_tokens, array('term' => $parent), $options, $bubbleable_metadata);
   +      $replacements += $token_service->generate('term', $parent_tokens, ['term' => $parent], $options, $bubbleable_metadata);
        }
   

   Whilst these changes look correct I'm also not sure that they are in scope for the issue.

Should cover everything from #77

+++ b/core/modules/taxonomy/taxonomy.tokens.inc
@@ -95,10 +95,15 @@ function taxonomy_tokens($type, $tokens, array $data, array $options, Bubbleable
+
+  $langcode = isset($options['langcode']) ? $options['langcode'] : $langcode = NULL;
+

This looks wrong... I think it should be $langcode = isset($options['langcode']) ? $options['langcode'] : NULLl. But looking at the node code don't we need to set the language option for term urls?

I think alexpott is right:

$langcode = isset($options['langcode']) ? $options['langcode'] : $langcode = NULL;

smells funny.
I tried it with the proposed

$langcode = isset($options['langcode']) ? $options['langcode'] : NULL;

and it worked for me.

+++ b/core/modules/taxonomy/taxonomy.tokens.inc
@@ -150,12 +154,12 @@ function taxonomy_tokens($type, $tokens, array $data, array $options, Bubbleable
   elseif ($type == 'vocabulary' && !empty($data['vocabulary'])) {
-    $vocabulary = $data['vocabulary'];
+    /** @var \Drupal\taxonomy\VocabularyInterface $vocabulary */
+    $vocabulary = \Drupal::entityManager()->getTranslationFromContext($data['vocabulary'], $langcode, ['operation' => 'vocabulary_tokens']);
 
     foreach ($tokens as $name => $original) {

config entities don't support this API, so this doesn't actually do anything.

it is more complicated for those, you need to switch the config translation override language and then switch it back, see user_mail()

Sorry - I just tested the 'term:parent' token. That worked.

Going to look at this today

OK, so did a reroll. And tried to address comments in #80 and #83.

Trying to find a way to test my changes manually, but let's see if it passes first.

Hm, feel like this is more correct.

The token replacements works as I expect, but not sure about if I am testing all cases.

Hello, guys!

The patch #89 was no longer cleanly applying to 8.5.x so I attach its reroll. Had to manually update a few things, mostly it was about short-array syntax and a couple of test files moving from simpletest to phpunit. As far as functionality, I have changed no thing.

Triggering test bots.

I am not sure why it failed. I ran that test locally through phpunit and through run-tests.sh both times successfully. Let me requeue it in case it was one-off failure.

ehmmm, I am not sure why it fails, the error message really says nothing particular and it does pass on my localhost. At the moment I do not have time to look into it.

15:24:32 Fatal error: Trait 'Drupal\system\Tests\Utility\AssertTokenReplacementTrait' not found in /var/www/html/core/modules/taxonomy/tests/src/Functional/TokenReplaceTest.php on line 19

This info might help from the jenkins console output. I don't have my dev setup currently to check it out more.

In fact AssertTokenReplacementTrait does not exist in core, and has been removed from patch #89 to patch #94.
Should be much better.

The interdiff was missing

Thanks @pguillard it's working fine on Drupal 8.5.1

@pguillard I probably found a new buggy case. Let say I have a vocabulary like this :

Vegetables

• Potatoes
• Eggplants

They're all translated this way :

Légumes

• Pommes de terre
• Aubergines

With the patch provided by @pguillard, If I save the children then the problem is solved. But If I save the parent term, then the problem is still there (Drupal 8.6.10). I save term "Vegetables" and then I have url updated like this : /fr/ingredients/vegetables/pommes-de-terre.html

Any clue ?

Rerolling against 8.8.x

#104:

I save term "Vegetables" and then I have url updated like this : /fr/ingredients/vegetables/pommes-de-terre.html

Any clue ?

Are you sure you don't have Flag other translations as outdated is checked?

Fixing deprecation errors.

Added some more tags on the issue to improve visibility, as this is still relevant in late 2019.

Tokens are used for all aliases in Drupal in the context of path-auto module, current usage reported in 150k+ at time of writing. When you have content translation and path-auto you will be getting wrong URL aliases due to this.

In the context of metatag module, you will be presenting wrongly translated information on the pages. This is another module with over 100k installations as of late 2019.

Hey together

The patch #107 isn't applying against 8.8.x
Do others have the same problem?

It was missing a file.

Works fine for me.

Looks like in #114 failed test not related to patch.
I've re-rolled patch against 9.1.x.
Let's see that testing bot say

Any chance to see a reroll of this in 8.x?

Probably as backport as of bug to 8.9.bugfix

This patch in comment 118 is working on Drupal 9 and is good to go!

This needs a reroll.

1. +++ b/core/modules/node/tests/src/Kernel/NodeTokenReplaceTest.php
   @@ -10,6 +10,8 @@
    use Drupal\Tests\system\Kernel\Token\TokenReplaceKernelTestBase;
    
    /**
   + * Generates text using placeholders.
   + *
     * Generates text using placeholders for dummy content to check node token
     * replacement.
     *
   
   +++ b/core/modules/taxonomy/tests/src/Functional/TokenReplaceTest.php
   @@ -5,8 +5,11 @@
    /**
   + * Generates text using placeholders.
   + *
   

   Let's not make these changes here. They are not in scope.

2. +++ b/core/modules/node/tests/src/Kernel/NodeTokenReplaceTest.php
   --- /dev/null
   +++ b/core/modules/system/src/Tests/Utility/AssertTokenReplacementTrait.php
   
   +++ b/core/modules/system/src/Tests/Utility/AssertTokenReplacementTrait.php
   +++ b/core/modules/system/src/Tests/Utility/AssertTokenReplacementTrait.php
   @@ -0,0 +1,84 @@
   
   @@ -0,0 +1,84 @@
   +<?php
   +
   +/**
   + * @file
   + * Contains \Drupal\system\Tests\Utility\AssertTokenReplacementTrait.
   + */
   

   Can this be moved to core/tests/Drupal/Tests/Traits/Core

3. +++ b/core/modules/system/src/Tests/Utility/AssertTokenReplacementTrait.php
   @@ -0,0 +1,84 @@
   + *
   + * Can be used by test classes that extend \Drupal\simpletest\WebTestBase.
   

   I think this line can be removed. It's wrong because WebTestBase is a thing of the past and I don't think we need to say this is for BrowserTestBase tests.

4. +++ b/core/modules/system/src/Tests/Utility/AssertTokenReplacementTrait.php
   @@ -0,0 +1,84 @@
   +   * @param array $tests
   +   *   Expected results keyed by their respective tokens.
   ...
   +   * @param array $metadata_tests
   +   *   The metadata to verify. Keyed by the relevant token.
   

   This construction of passing an array of tests and an array of metadata tests is confusing.

5. +++ b/core/modules/system/src/Tests/Utility/AssertTokenReplacementTrait.php
   @@ -0,0 +1,84 @@
   +   * @param $message
   +   *   The message to display with the assertion. Can contain replacement
   +   *   tokens:
   +   *   - %token for the token name
   +   *   - %output for the value returned by the token replacement service
   +   *   - %expected for the expected result
   

   I'm not sure about passing in the message here. I think we could build the message here using the $token and the key of $data.

6. +++ b/core/modules/system/src/Tests/Utility/AssertTokenReplacementTrait.php
   @@ -0,0 +1,84 @@
   +      $this->assertEqual($output, new HtmlEscapedText($expected), new FormattableMarkup($message, [
   +        '%token' => $token,
   +        '%output' => $output,
   +        '%expected' => $expected,
   +      ]));
   +
   +      $this->assertEqual($bubbleable_metadata, $metadata_tests[$token]);
   

   $this->assertEquals() and the expected value should go first.

7. +++ b/core/modules/system/src/Tests/Utility/AssertTokenReplacementTrait.php
   @@ -0,0 +1,84 @@
   +      $this->assertEqual($bubbleable_metadata, $metadata_tests[$token]);
   

   If this fails then it be helpful to have a message saying which token it was for. See above point about the $message param. I think we can build a message for each assertion in the test and it'll be better.

8. +++ b/core/modules/system/src/Tests/Utility/AssertTokenReplacementTrait.php
   @@ -0,0 +1,84 @@
   +  /**
   +   * Asserts if tokens are correctly replaced.
   +   *
   +   * @param array $tests
   +   *   Expected results keyed by their respective tokens.
   +   * @param array $data
   +   *   The data to perform the replacement on. @see Token::replace().
   +   * @param array $options
   +   *   Additional replacement options. @see Token::replace().
   +   * @param $message
   +   *   The message to display with the assertion. Can contain replacement
   +   *   tokens:
   +   *   - %token for the token name
   +   *   - %output for the value returned by the token replacement service
   +   *   - %expected for the expected result
   +   */
   +  protected function assertTokenReplacement(array $tests, array $data, array $options, $message) {
   +    foreach ($tests as $token => $expected) {
   +      $output = \Drupal::token()->replace($token, $data, $options);
   +      $expected = ($expected instanceof MarkupInterface) ? $expected : new HtmlEscapedText($expected);
   +      $this->assertEqual($output, $expected, new FormattableMarkup($message, [
   +        '%token' => $token,
   +        '%output' => $output,
   +        '%expected' => $expected,
   +      ]));
   +    }
   +  }
   

   The comments about the test method above apply here too.

9. +++ b/core/modules/taxonomy/tests/src/Functional/TokenReplaceTest.php
   @@ -103,37 +122,67 @@ public function testTaxonomyTokenReplacement() {
        // Generate and test sanitized tokens for term2.
        $tests = [];
        $tests['[term:tid]'] = $term2->id();
        $tests['[term:name]'] = $term2->getName();
   -    $tests['[term:description]'] = $term2->description->processed;
   +    $tests['[term:description]'] = $term2->getDescription();
        $tests['[term:url]'] = $term2->toUrl('canonical', ['absolute' => TRUE])->toString();
        $tests['[term:node-count]'] = 1;
   +    $tests['[term:vocabulary]'] = $this->vocabulary->label();
   +    $tests['[term:parent]'] = $term1->getName();
   +    $tests['[term:parent:tid]'] = $term1->id();
        $tests['[term:parent:name]'] = $term1->getName();
   +    $tests['[term:parent:description]'] = $term1->getDescription();
        $tests['[term:parent:url]'] = $term1->toUrl('canonical', ['absolute' => TRUE])->toString();
   +    $tests['[term:parent:node-count]'] = 0;
        $tests['[term:parent:parent:name]'] = '[term:parent:parent:name]';
        $tests['[term:vocabulary:name]'] = $this->vocabulary->label();
   +    $tests['[term:parent:vocabulary]'] = $this->vocabulary->label();
   +    $tests['[term:parent:vocabulary:name]'] = $this->vocabulary->label();
    
        // Test to make sure that we generated something for each token.
        $this->assertFalse(in_array(0, array_map('strlen', $tests)), 'No empty tokens generated.');
    
   -    foreach ($tests as $input => $expected) {
   -      $output = $token_service->replace($input, ['term' => $term2], ['langcode' => $language_interface->getId()]);
   -      $this->assertEqual($output, $expected, new FormattableMarkup('Sanitized taxonomy term token %token replaced.', ['%token' => $input]));
   -    }
   +    $base_bubbleable_metadata = BubbleableMetadata::createFromObject($term2);
   +    $metadata_tests = [];
   +    $metadata_tests['[term:tid]'] = $base_bubbleable_metadata;
   +    $metadata_tests['[term:name]'] = $base_bubbleable_metadata;
   +    $metadata_tests['[term:description]'] = $base_bubbleable_metadata;
   +    $metadata_tests['[term:url]'] = $base_bubbleable_metadata;
   +    $metadata_tests['[term:node-count]'] = $base_bubbleable_metadata;
   +    $bubbleable_metadata = clone $base_bubbleable_metadata;
   +    $bubbleable_metadata = $bubbleable_metadata->addCacheTags($this->vocabulary->getCacheTags());
   +    $metadata_tests['[term:vocabulary]'] = $bubbleable_metadata;
   +    $metadata_tests['[term:vocabulary:name]'] = $bubbleable_metadata;
   +    $base_bubbleable_metadata = BubbleableMetadata::createFromObject($term1)->addCacheTags($term2->getCacheTags());
   +    $metadata_tests['[term:parent]'] = $base_bubbleable_metadata;
   +    $metadata_tests['[term:parent:tid]'] = $base_bubbleable_metadata;
   +    $metadata_tests['[term:parent:name]'] = $base_bubbleable_metadata;
   +    $metadata_tests['[term:parent:description]'] = $base_bubbleable_metadata;
   +    $metadata_tests['[term:parent:url]'] = $base_bubbleable_metadata;
   +    $metadata_tests['[term:parent:node-count]'] = $base_bubbleable_metadata;
   +    $metadata_tests['[term:parent:parent:name]'] = $base_bubbleable_metadata;
   +    $bubbleable_metadata = clone $base_bubbleable_metadata;
   +    $bubbleable_metadata = $bubbleable_metadata->addCacheTags($this->vocabulary->getCacheTags());
   +    $metadata_tests['[term:parent:vocabulary:name]'] = $bubbleable_metadata;
   +    $metadata_tests['[term:parent:vocabulary]'] = $bubbleable_metadata;
   +
   +    $data = ['term' => $term2];
   +    $msg = 'Taxonomy term 2 token %token replaced with %output which is equal to %expected';
   +    $this->assertTokenReplacementAndCheckMetadata($tests, $data, $options, $msg, $metadata_tests);
    
   

   Separating the test and metadata test set-up makes this harder to think about than it needs to be.

   Yes this is an existing problem but we're now making an API because we're adding the trait. I think we should consider ditching the trait here and file a follow-up to make a nicer trait. With a clearer API. It would be nice if each test passed to the trait looked something like

   [
     'token' => '[term:parent:name]',
     'expected_value' => 'Term Name'
     'expected_metadata => object
   ]
   

Hey together

The patch in #114 isn't applying to 8.9.1 anymore. Looks like it needs a reroll?

Rerolled the patch from #114 against 8.9.x.

Working on this.

Here I have added re-roll of patch #118.

I have also fixed points 1, 2, 3 and 6 of comment #122.

It still needs work for the remaining points of comment #122.

Working on this.

Fixed use statement issue of patch #126.

Rerolled the patch in #128, thanks!

Updated patch.

I needed the patch against the 9.1.5 version. Here the rerolled patch.

Rerolled the patch #132

fixed the failed test case.

1. +++ b/core/modules/node/node.tokens.inc
   @@ -183,15 +181,14 @@ function node_tokens($type, $tokens, array $data, array $options, BubbleableMeta
    
            case 'created':
   -          $date_format = DateFormat::load('medium');
   -          $bubbleable_metadata->addCacheableDependency($date_format);
   -          $replacements[$original] = \Drupal::service('date.formatter')->format($node->getCreatedTime(), 'medium', '', NULL, $langcode);
   -          break;
    
            case 'changed':
   +          /** @var \Drupal\Core\Datetime\Entity\DateFormat $date_format */
              $date_format = DateFormat::load('medium');
   +          $date_raw = ($name === 'created' ? $node->getCreatedTime() : $node->getChangedTime());
   +          $date_formatted = \Drupal::service('date.formatter')->format($date_raw, $date_format->id(), '', NULL, $langcode);
              $bubbleable_metadata->addCacheableDependency($date_format);
   -          $replacements[$original] = \Drupal::service('date.formatter')->format($node->getChangedTime(), 'medium', '', NULL, $langcode);
   +          $replacements[$original] = $date_formatted;
              break;
          }
   

   this refactoring seems unrelated and only saves a single line of code at the cost of making the patch bigger and the code harder to understand. I'd suggest to revert these changes.

2. +++ b/core/modules/taxonomy/taxonomy.tokens.inc
   @@ -95,8 +95,12 @@ function taxonomy_tokens($type, $tokens, array $data, array $options, Bubbleable
   +
      if ($type == 'term' && !empty($data['term'])) {
   -    $term = $data['term'];
   +    /** @var \Drupal\taxonomy\TermInterface $term */
   +    $term = \Drupal::service('entity.repository')->getTranslationFromContext($data['term'], $langcode, ['operation' => 'term_tokens']);
    
        foreach ($tokens as $name => $original) {
   

   I know this is copied from node tokens, but the problem is that the operations argument is weird and content_translation only implements it for the default entity_view operation. So I'd suggest to leave out the argument, we might deprecate that thing.

   See #2978048: Unpublished translations should fallback to the original language

3. +++ b/core/modules/taxonomy/taxonomy.tokens.inc
   @@ -109,9 +113,7 @@ function taxonomy_tokens($type, $tokens, array $data, array $options, Bubbleable
            case 'description':
   -          // "processed" returns a \Drupal\Component\Render\MarkupInterface via
   -          // check_markup().
   -          $replacements[$original] = $term->description->processed;
   +          $replacements[$original] = $term->getDescription();
   

   This too is unrelated refactoring and it's not the same. getDescription() returns the value, not the processed string.

4. +++ b/core/modules/taxonomy/taxonomy.tokens.inc
   @@ -158,6 +161,11 @@ function taxonomy_tokens($type, $tokens, array $data, array $options, Bubbleable
      elseif ($type == 'vocabulary' && !empty($data['vocabulary'])) {
   +    /** @var \Drupal\taxonomy\VocabularyInterface $vocabulary */
   +    $language_manager = \Drupal::languageManager();
   +    $language = $language_manager->getLanguage($langcode);
   +    $original_language = $language_manager->getConfigOverrideLanguage();
   +    $language_manager->setConfigOverrideLanguage($language);
        $vocabulary = $data['vocabulary'];
   

   I'm pretty sure this does nothing.

   The API for config entities is very different and applies to when they are loaded. the vocabulary is already loaded here. This sadly needs to be done whenever token replace is called and the vocabulary is loaded. For example the term:vocabulary token.

5. +++ b/core/modules/taxonomy/tests/src/Functional/TokenReplaceTest.php
   @@ -14,6 +14,8 @@
    class TokenReplaceTest extends TaxonomyTestBase {
    
   +  use AssertTokenReplacementTrait;
   +
      /**
   

   I think @alexpott proposed to not introduce this trait here, makes the test harder to understand.

6. +++ b/core/modules/taxonomy/tests/src/Functional/TokenReplaceTest.php
   @@ -68,12 +80,11 @@ protected function setUp(): void {
    
   -    // Create two taxonomy terms.
   -    $term1 = $this->createTerm($this->vocabulary);
   -    $term2 = $this->createTerm($this->vocabulary);
   +    // Create two taxonomy terms with unsafe names.
   +    $term1 = $this->createTerm($this->vocabulary, ['name' => 'T1 <script>"&lt;name&gt;"</script>']);
   +    $term2 = $this->createTerm($this->vocabulary, ['name' => 'T2 <strong>"&lt;name&gt;"</strong>']);
   

   how is escaping or unsafe names related to translating them? Is it possible this is a rebase of a very old version of this patch that still had that. I think we can drop all these changes.

7. +++ b/core/modules/taxonomy/tests/src/Functional/TokenReplaceTest.php
   @@ -93,13 +104,18 @@ public function testTaxonomyTokenReplacement() {
        $tests['[term:name]'] = $term1->getName();
   -    $tests['[term:description]'] = $term1->description->processed;
   +    $tests['[term:description]'] = $term1->getDescription();
        $tests['[term:url]'] = $term1->toUrl('canonical', ['absolute' => TRUE])->toString();
   

   this needs to be reverted too.

8. +++ b/core/modules/taxonomy/tests/src/Functional/TokenReplaceTest.php
   @@ -93,13 +104,18 @@ public function testTaxonomyTokenReplacement() {
    
   +    // Test to make sure that we generated something for each token.
   +    $this->assertFalse(in_array(0, array_map('strlen', $tests)), 'No empty tokens generated.');
   +
   

   this is testing the test, I'm not sure what this is for. remove?

9. +++ b/core/modules/taxonomy/tests/src/Functional/TokenReplaceTest.php
   @@ -108,37 +124,68 @@ public function testTaxonomyTokenReplacement() {
        $tests['[term:name]'] = $term2->getName();
        $tests['[term:description]'] = $term2->description->processed;
   +    $tests['[term:description]'] = $term2->getDescription();
        $tests['[term:url]'] = $term2->toUrl('canonical', ['absolute' => TRUE])->toString();
        $tests['[term:node-count]'] = 1;
   +    $tests['[term:vocabulary]'] = $this->vocabulary->label();
   +    $tests['[term:parent]'] = $term1->getName();
   +    $tests['[term:parent:tid]'] = $term1->id();
        $tests['[term:parent:name]'] = $term1->getName();
   +    $tests['[term:parent:description]'] = $term1->getDescription();
        $tests['[term:parent:url]'] = $term1->toUrl('canonical', ['absolute' => TRUE])->toString();
   +    $tests['[term:parent:node-count]'] = 0;
        $tests['[term:parent:parent:name]'] = '[term:parent:parent:name]';
        $tests['[term:vocabulary:name]'] = $this->vocabulary->label();
   +    $tests['[term:parent:vocabulary]'] = $this->vocabulary->label();
   +    $tests['[term:parent:vocabulary:name]'] = $this->vocabulary->label();
    
   

   Beside the refactoring and some more tokens, I don't see how the fixes are actually asserted here.

   No translations are created, no language options are passed. to the test.

   To be honest, I would consider to just start from scratch with the test. As a bugfix, this is actually a good candidate for writing failing tests first. add the language modules, add a second language, add translations to the term and then fetch the same tokens with different options to get the translated values.

Does anyone have a patch that works against 8.9.18?

I created a merge request to understand better the changes and to be more clear with the review, we need this for our project.

Update patch to 9.5 in work