When trying to do the spanish translation of the pot file, I realiced several issues:

I was adviced about the following errors:

* The first parameter to t() should be a literal string. There should be no variables, concatenation, constants or other non-literal strings there. At t(variable_get('login_security_host_hard_banned',LOGIN_SECURITY_HOST_HARD_BANNED),$variables) in login_security.module on line 370.
* The first parameter to t() should be a literal string. There should be no variables, concatenation, constants or other non-literal strings there. At t(variable_get('login_security_user_blocked',LOGIN_SECURITY_USER_BLOCKED),$variables) in login_security.module on line 382.

And I saw strings that could be reformatted:

#: login_security.install:26
msgid "The ip address of the connection."
#: login_security.install:101
msgid "The IP address of the connection."

#: login_security.install:33
msgid "Username used in the login."
#: login_security.install:129
msgid "Username used in the login submission."

I'll use the same case and reduce the number of possible string combinations, as I do update the new .pot file.

CommentFileSizeAuthor
#4 500740_strings_consolidation_and_t_issues_1.patch10.72 KBilo
#2 500740_strings_consolidation_and_t_issues.patch10.22 KBilo
#2 500740_new_pot_file.patch9.96 KBilo

Comments

deekayen’s picture

deekayen commented

I previously noticed and fixed some instances where t() was wrapped around variable_gets(). Those are now strtr() instead. Example:

      // this loop is instead of doing t() because t() can only translate static strings, not variables.
      foreach ($variables as $key => $value) {
        $variables[$key] = theme('placeholder', $value);
      }
      form_set_error('submit', strtr(variable_get('login_security_host_soft_banned',  LOGIN_SECURITY_HOST_SOFT_BANNED), $variables));

Since there are so many, that might also benefit from a separate function so it'd turn into something like (untested):

      form_set_error('submit', login_security_t(variable_get('login_security_host_soft_banned',  LOGIN_SECURITY_HOST_SOFT_BANNED), $variables));

/**
 * This option is instead of doing t() because t() can only translate static strings, not function returns.
 */
function login_security_t($message, $variables = array()) {
  foreach ($variables as $key => $value) {
    $variables[$key] = theme('placeholder', $value);
  }
  return strtr($message, $variables);
}
ilo’s picture

ilo commented
Status: Active » Needs review
StatusFileSize
new 500740_new_pot_file.patch9.96 KB
new 500740_strings_consolidation_and_t_issues.patch10.22 KB

Although the patch touches several files, the hard part was the login_security_t translator, now included is a replacement for t in the whole module. functionality test passes, and manual revision of the messages show correct strings.

deekayen’s picture

deekayen commented
Status: Needs review » Needs work

500740_strings_consolidation_and_t_issues.patch no longer applies cleanly.

ilo’s picture

ilo commented
Title: Review translatable strings and .pot template » Review translatable strings for login_security_t()
Component: User interface » Code
Priority: Normal » Critical
Status: Needs work » Needs review
StatusFileSize
new 500740_strings_consolidation_and_t_issues_1.patch10.72 KB

I rerolled the patch to current -dev status. I passed the tests and seems to work. I would like someone makes a quick review before committing. I just removed everything that has nothing to do with t() issues to clean the patch ewview a little bit. The rest of items (strings and .pot will go to another issue).

ilo’s picture

ilo commented
Status: Needs review » Fixed

All tests passed. No coder issues. Verified manually. Commited to 6.x-1.x-dev

Status: Fixed » Closed (fixed)

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

  • ilo committed 8b91a2a on 6.x-1.x, 8.x-1.x 
    #500740 by deekayen, ilo: consolidate similar strings, and remove...