While working on #554088: Move user_mail_tokens() and emails to token system I ran into problems with undefined variables when using token_replace

From user.tokens.inc:

function user_tokens($type, $tokens, array $data = array(), array $options = array()) {
  global $user;
  $url_options = array('absolute' => TRUE);
  if (isset($options['language'])) {
    $url_options['language'] = $language;
    $language_code = $language->language;
  }
  else {
    $language_code = NULL;
  }

When token_replace is called with 'language' defined in the $options-array, this will result in a PHP notice: undefined variable language.

The same approach is used for comment_tokens and node_tokens.

CommentFileSizeAuthor
#7 drupal.token-undefined-language.patch4.2 KBsun
#5 drupal.token-language-test.patch1.6 KBsun
token-undefined-language.patch2.46 KBmr.baileys

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community

Awesome.

dries’s picture

dries commented

We should probably improve our token tests in ./modules/system/system.test?

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

uhm.

ok, I understand that we don't have any tests for tokens at all yet.

But with a request for language dependent tests you're even screwing up me. :P

For that... let me brainstorm...

We'd need a system object that's available in two languages. Menus and system variables don't count, as Drupal core doesn't support translation/localization for them. (which still is totally wonky)

Leaving us with nodes, comments, and users. Well, comments and users are hard to translate as well.

Leaving us with nodes.

Hence, a test that installs Translation module, creates a node, translates that into another language, and uses the node title to check whether tokens are generated properly.

Ugh.

dries’s picture

dries commented

Sorry, I assumed that we did have some tests in ./modules/system/system.test and that it would be relatively easy to add a test for this. If that is a false assumption, I'm happy to proceed without tests for now.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new drupal.token-language-test.patch1.6 KB

Somehow, my grep for "token" didn't find "TokenReplaceTestCase".

So this patch should fail.

Status: Reviewed & tested by the community » Needs work

The last submitted patch failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new drupal.token-undefined-language.patch4.2 KB

Alright. And merging both patches should pass. :)

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed. Thanks.

Status: Fixed » Closed (fixed)

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