While #334283: Add msgctxt-type context to t() added context to t(), format_plural() and friends in PHP, it did not add it to their counterparts in JavaScript. Since we tried to keep the APIs in sync, and JS code in Drupal modules and themes is on the increase still, this looks like a bad omission. To add JS support, we should modify the code in _locale_parse_js_file(). But first, we need to define the API for it. While we worked hard to keep the API the same as in PHP (minus the $langcode) parameter, the new conversion of $langcode to $options which can also contain a context key dictates that we add the similar options object parameter here, which might not be easy to parse via regular expressions. Maybe we need to grab the optional options object and convert it as JSON to the PHP data structure.

JS parsing currently happens this way:

<?php
/**
* Parses a JavaScript file, extracts strings wrapped in Drupal.t() and
* Drupal.formatPlural() and inserts them into the database.
*/
function _locale_parse_js_file($filepath) {
  global
$language;
 
// Load the JavaScript file.
 
$file = file_get_contents($filepath);
 
// Match all calls to Drupal.t() in an array.
  // Note: \s also matches newlines with the 's' modifier.
 
preg_match_all('~[^\w]Drupal\s*\.\s*t\s*\(\s*(' . LOCALE_JS_STRING . ')\s*[,\)]~s', $file, $t_matches);
 
// Match all Drupal.formatPlural() calls in another array.
 
preg_match_all('~[^\w]Drupal\s*\.\s*formatPlural\s*\(\s*.+?\s*,\s*(' . LOCALE_JS_STRING . ')\s*,\s*((?:(?:\'(?:\\\\\'|[^\'])*@count(?:\\\\\'|[^\'])*\'|"(?:\\"|[^"])*@count(?:\\"|[^"])*")(?:\s*\+\s*)?)+)\s*[,\)]~s', $file, $plural_matches);
 
// Loop through all matches and process them.
 
$all_matches = array_merge($plural_matches[1], $t_matches[1]);
  foreach (
$all_matches as $key => $string) {
   
$strings = array($string);
   
// If there is also a plural version of this string, add it to the strings array.
   
if (isset($plural_matches[2][$key])) {
     
$strings[] = $plural_matches[2][$key];
    }
    foreach (
$strings as $key => $string) {
     
// Remove the quotes and string concatenations from the string.
     
$string = implode('', preg_split('~(?<!\\\\)[\'"]\s*\+\s*[\'"]~s', substr($string, 1, -1)));
     
$source = db_query("SELECT lid, location FROM {locales_source} WHERE source = :source AND textgroup = 'default'", array(':source' => $string))->fetchObject();
      if (
$source) {
       
// We already have this source string and now have to add the location
        // to the location column, if this file is not yet present in there.
       
$locations = preg_split('~\s*;\s*~', $source->location);
        if (!
in_array($filepath, $locations)) {
         
$locations[] = $filepath;
         
$locations = implode('; ', $locations);
         
// Save the new locations string to the database.
         
db_update('locales_source')
            ->
fields(array(
             
'location' => $locations,
            ))
            ->
condition('lid', $source->lid)
            ->
execute();
        }
      }
      else {
       
// We don't have the source string yet, thus we insert it into the database.
       
db_insert('locales_source')
          ->
fields(array(
           
'location' => $filepath,
           
'source' => $string,
           
'context' => '',
           
'textgroup' => 'default',
          ))
          ->
execute();
      }
    }
  }
}
?>
Files: 
CommentFileSizeAuthor
#39 js-t-context-488496-39.patch15.15 KBloganfsmyth
PASSED: [[SimpleTest]]: [MySQL] 36,951 pass(es).
[ View ]
#33 js-t-context-488496-33.patch14.68 KBloganfsmyth
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch js-t-context-488496-33.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#28 js-t-context-488496-28.patch14.66 KBloganfsmyth
PASSED: [[SimpleTest]]: [MySQL] 33,590 pass(es).
[ View ]
#26 js-t-context-488496-26.patch14.43 KBloganfsmyth
PASSED: [[SimpleTest]]: [MySQL] 33,373 pass(es).
[ View ]
#25 js-t-context-488496-25.patch14.43 KBloganfsmyth
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch js-t-context-488496-25.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#23 js-t-context-488496-23.patch9.57 KBloganfsmyth
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch js-t-context-488496-23_0.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#22 javascript-t-context-support-488496-22.patch9.3 KBloganfsmyth
PASSED: [[SimpleTest]]: [MySQL] 32,873 pass(es).
[ View ]
#20 javascript-t-context-support-488496-20.patch9.29 KBloganfsmyth
FAILED: [[SimpleTest]]: [MySQL] 33,145 pass(es), 46 fail(s), and 14 exception(es).
[ View ]

Comments

Issue tags:+msgctx

Tagged to summarize all contexts

Subscribe.
Need this for the d7 module upgrade.

Wow, _locale_parse_js_file does look like a pile of gross.

Component:base system» javascript
Priority:Normal» Critical

Suppose this issue should be critical because now this functionality is different from core's

Assigned:Unassigned» cwgordon7

I can probably do this.

Assigned:cwgordon7» Unassigned

Ok, this is way more complicated than I thought it would be. Parsing JavaScript through a giant regular expression to find strings to translate is fine, but when I have to modify that regular expression to additionally match a JavaScript object, I'm stuck. If someone can write me a regular expression to match the JavaScript object, I can finish this up.

@cwgordon7 what for you trying to add JS-objects? I think it could be just another param for Drupal.t() Please, provide some example

@andypost: The additional parameter for Drupal.t would be the JavaScript object. Note that in JavaScript, an array with string keys is considered an object. It's just beyond me how to match a JS-object in a PCRE. Consider the current code:

<?php
 
// Match all calls to Drupal.t() in an array.
  // Note: \s also matches newlines with the 's' modifier.
 
preg_match_all('~[^\w]Drupal\s*\.\s*t\s*\(\s*(' . LOCALE_JS_STRING . ')\s*[,\)]~s', $file, $t_matches);
 
// Match all Drupal.formatPlural() calls in another array.
 
preg_match_all('~[^\w]Drupal\s*\.\s*formatPlural\s*\(\s*.+?\s*,\s*(' . LOCALE_JS_STRING . ')\s*,\s*((?:(?:\'(?:\\\\\'|[^\'])*@count(?:\\\\\'|[^\'])*\'|"(?:\\"|[^"])*@count(?:\\"|[^"])*")(?:\s*\+\s*)?)+)\s*[,\)]~s', $file, $plural_matches);
?>

I have no idea how to change that to additionally match and store the value of a JavaScript object's (var name options) 'context' key.

Sub

A JavaScript object should be valid JSON, so if it makes it easier, you can match the full object and pass it through drupal_json_decode to get a PHP array - rather than trying to parse it in with regex.

Priority:Critical» Normal

is this really critical?

Priority:Normal» Critical

I'm not familiar with the JS object that's being parsed here, but it's worth noting that JSON is only a subset of valid JS object notation - you can have perfectly valid JS defining a perfectly valid JS object, and JSON parsers will choke on it. There are full Javascript parsers out there, such as http://web.2point1.com/2009/11/14/jparser-and-jtokenizer-released/ - I would assume that's overkill here, but I'm throwing it out there just in case.

Priority:Critical» Normal

Oops.

Priority:Normal» Major

Suppose this major and maybe critical for release because it's functionality exists in D6 but broken in D7

Actually this needs a regexp guru

Well, its not at all functionality that exists in Drupal 6, but it is indeed missing functionality that exists in PHP in Drupal 7 but not in JS in Drupal 7. As I said in the issue starter and as others pointed out, we can grab a whole object and parse it as JSON, so we can grab the context.

Version:7.x-dev» 8.x-dev

Well, Drupal 7 will not be consistent in context support, we can try in Drupal 8.

Is there a reason why this cannot be fixed in 7.1?
While you might technically call this an API change, I don't see any BC-break here.

Issue tags:+D8MI

Tag for Drupal 8 Multilingual Initiative.

Assigned:Unassigned» loganfsmyth

Assigned:loganfsmyth» Unassigned
Status:Active» Needs review
StatusFileSize
new9.29 KB
FAILED: [[SimpleTest]]: [MySQL] 33,145 pass(es), 46 fail(s), and 14 exception(es).
[ View ]

Here is a first attempt at this.

I have added an 'options' argument as the last argument of Drupal.t() and Drupal.formatPlural(), and updated the JS and PHP to make Drupal.locale.strings be nested by context, then source string, instead of just source string. This should all be similar to the way the core t() function works.

As for the complicated part, I have changed the regular expressions to match the context argument out of the options object. I did not use json_decode in the end because I decided that it would be too difficult to process the object literal as JSON, since something like {context:"mycontext"} will fail. I also thought it would be too confusing for developers to always have to make sure that particular argument was JSON compliant.

This seems to work overall, and the only matching problems that I can see that may need to be discussed as the following:
* 'args' has to be matched as object literal since anything else would probably be too strong. This means that if a user did Drupal.t('Some string', undefined, {context: 'kjdfg'}); it would not match.
* I don't think this will come up, but it's important to note that any nested objects in those literals could break parsing if they are in the right place.

Status:Needs review» Needs work

The last submitted patch, javascript-t-context-support-488496-20.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new9.3 KB
PASSED: [[SimpleTest]]: [MySQL] 32,873 pass(es).
[ View ]

Here is a version that should pass testing :P Regular expressions are COMPLICATED.

StatusFileSize
new9.57 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch js-t-context-488496-23_0.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

This new patch relies on #1281932: Convert Drupal.t and Drupal.formatPlural regular expressions to extended form..

It uses extended regexes to make life easier for everyone.

Status:Needs review» Needs work

The last submitted patch, js-t-context-488496-23.patch, failed testing.

StatusFileSize
new14.43 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch js-t-context-488496-25.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Here is a new patch that adds some tests to go along with my newest patch in #1281932: Convert Drupal.t and Drupal.formatPlural regular expressions to extended form., and removes some unneeded whitespace checks from the context regex.

Status:Needs work» Needs review
StatusFileSize
new14.43 KB
PASSED: [[SimpleTest]]: [MySQL] 33,373 pass(es).
[ View ]

Status:Needs review» Needs work

Wow, thanks for keeping this up! This looks *very* good. Just two comments:

- phpdoc comment blocks should have one line summary (noticed for the two define()'s you put in) and then the details after an empty line
- it would be great to add test coverage where arguments (replacements for the string) are provided before context, so we can ensure that works too

Otherwise it all looks good to me.

Status:Needs work» Needs review
StatusFileSize
new14.66 KB
PASSED: [[SimpleTest]]: [MySQL] 33,590 pass(es).
[ View ]

Fixed those comments and added 3 more testing strings, with substitution arguments.

Looks fabulous! I think if this passes tests, it should be good to go :)

Issue tags:+needs backport to D7

This should be backported, right?

Title:Implement msgctx context in JS translationImplement missing msgctx context in JS translation for feature parity with t()
Status:Needs review» Reviewed & tested by the community

It is definitely good for D8, I think it would be great for D7 and the tests should prove the old (limited) parsing behavior still works too, so it should not be an issue hopefully to get into D7 too.

Status:Reviewed & tested by the community» Needs work

+++ b/includes/locale.incundefined
@@ -43,6 +43,37 @@ define('LOCALE_LANGUAGE_NEGOTIATION_SESSION', 'locale-session');
+ * This pattern matches a basic JS object, but will fail on an object with
+ * nested objects. Used in JS file parsing for.

for.... what?

+++ b/includes/locale.incundefined
@@ -43,6 +43,37 @@ define('LOCALE_LANGUAGE_NEGOTIATION_SESSION', 'locale-session');
+ * Pattern to match a JS object containing a 'context key'
+ * with a string value, which is captured. Will fail if there

Very minor but these lines are way under 80 chars, would likely fit on two lines if they used the space better.

Otherwise looks good to me and thanks for the extensive tests!
-19 days to next Drupal core point release.

Status:Needs work» Needs review
StatusFileSize
new14.68 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch js-t-context-488496-33.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Good call. That's what I get for working on a patch on a plane.

I made it actually say what that pattern is for, and made the other comment wrap at 80 so it is 2 lines instead of 3.

Status:Needs review» Reviewed & tested by the community

Passes again, fixed issues called out by @catch and otherwise universally liked, so should be good to go :)

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Needs review

Committed and pushed to 8.x, thanks! Moving to 7.x for consideration.

Issue tags:-msgctx, -needs backport to D7, -D8MI

#33: js-t-context-488496-33.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+msgctx, +needs backport to D7, +D8MI

The last submitted patch, js-t-context-488496-33.patch, failed testing.

Added this change notice http://drupal.org/node/1323152, thanks!

Status:Needs work» Needs review
StatusFileSize
new15.15 KB
PASSED: [[SimpleTest]]: [MySQL] 36,951 pass(es).
[ View ]

Rerolled against D7. Only changes from D8 version were adding back in textgroup='default' conditions.

Status:Needs review» Reviewed & tested by the community

Looks good to me. Plugs in functionality we did not get to with Drupal 7.0. Expands tests nicely, so looks ready to go.

Status:Reviewed & tested by the community» Fixed

Hm. This patch looks slightly terrifying. :) However, looking at the change notice, this is only adding new functionality to Drupal.t(), not changing any APIs, so should be good to go.

Committed and pushed to 7.x. Thanks!

Status:Fixed» Closed (fixed)

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

Issue tags:+language-ui

Tagging for UI translation leg of D8MI.