Allow checkout completion message to be translated

I wasn't aware of translation issues when HTML is included in the string. Is there no way to change which HTML tags are allowed? The problem with taking the a tag out is that I then have to use a token for the whole link, which is much less flexible than just using a token for the URL and putting whatever I want as the link text.

Subscribing

+1

subscribe

subscribe

#928948: _filter_xss_attributes should ignore attribute values for "value" and "flashvars"

We should find a workable solution for this. It's not really an option to wait for this patch mentioned by Heine to land in D8 (and then hopefully get backported to D7).
Here is my attempt at fixing this. It makes the URL more hardcoded, but that seems the only option.

this not function for me.

Subscribe

I don't think this is a core bug. This is just a feature of t(). You need to read the D6 API documentation to find out what is does and why. The proposed patch is the way to go. In URLs no "dangerous" signs should appear like the colon in square brackets. The way to go is to remove the token and replace it by a place holder.

Why not just use the l function with a variable_get for the path and leave it to the administrator where to link afterwards (with a default to )?

A link back to the frontpage isn't very helpful anyway imho..

@svendecabooter
why is [commerce-order:order-number] still in the message ?
it applies cleanly but it does not work
still get the message complaining about HTML !!

temporarily I change locale.inc/locale_string_is_safe($string) and add a return true

GiorgosK: which version of Commerce and Drupal core are you using?

The error about HTML I encountered was specifically caused by a : symbol being placed within double quotes (").
Since the [commerce-order:order-number] variable is not within an HTML link ( tag), it works perfectly here.
I'm not running the latest Commerce code though, so maybe something introduced your problems there. I should test this out when I find some time.

I applied the patch with the following result:

    Warning: Missing argument 1 for commerce_checkout_completion_message_default(), called in /srv/www/htdocs/myhome/mydomain/sites/all/modules/commerce/modules/checkout/includes/commerce_checkout.checkout_pane.inc on line 132 and defined in commerce_checkout_completion_message_default() (line 880 of /srv/www/htdocs/myhome/mydomain/sites/all/modules/commerce/modules/checkout/commerce_checkout.module).
    Notice: Undefined variable: order in commerce_checkout_completion_message_default() (line 881 of /srv/www/htdocs/myhome/mydomain/sites/all/modules/commerce/modules/checkout/commerce_checkout.module).
    Notice: Trying to get property of non-object in commerce_checkout_completion_message_default() (line 881 of /srv/www/htdocs/myhome/mydomain/sites/all/modules/commerce/modules/checkout/commerce_checkout.module).
    Notice: Undefined variable: order in commerce_checkout_completion_message_default() (line 881 of /srv/www/htdocs/myhome/mydomain/sites/all/modules/commerce/modules/checkout/commerce_checkout.module).
    Notice: Trying to get property of non-object in commerce_checkout_completion_message_default() (line 881 of /srv/www/htdocs/myhome/mydomain/sites/all/modules/commerce/modules/checkout/commerce_checkout.module).

with the link to the order missing the order id.

For the moment I'll simply avoid the tokens with colons in the translation.

I think this would solve i18n issue most effectively.

I had a problem with this phrase, but it solved.

<a href="[site:url]">Return to the front page.</a>

+++ b/modules/checkout/commerce_checkout.module
@@ -875,6 +875,5 @@ function commerce_checkout_views_api() {
+  return t('Your order is number !ordernumber. You can <a href="!orderurl">view your order</a> on your account page when logged in.', array('!ordernumber' => token_replace('[commerce-order:order-number]'), '!orderurl' => token_replace('[commerce-order:url]'))) . "\n\n" . t('<a href="!fronturl">Return to the front page.</a>', array('!fronturl' => token_replace('[site:url]')));

All placeholders in there have to use @.

But !orderurl and !fronturl must be inserted as they are, since htmlspecialchars would render link unusable. Agreed for !ordernumber, it can be @ordernumber.

All HTML attribute values need to go through check_plain(), hence you have to use the @-placeholder.

You can find many examples for this throughout Drupal core. (Admittedly not for token_replace(), but your tokens here seem to produce straightforward/raw URIs only, so there's no difference to usage of t().)

Sun, I tested and you are right. Here's fixed patch.

The only thing I don't like about this approach is that those tokens are useless; as soon as someone goes to the edit form for the checkout completion message and tweaks it, the tokens will no longer be guiding the message displayed. This could open the doors for unintended side effects in cases of multiple development environments. But it's either that or define our own pseudo-tokens for use in the checkout completion message. Need to weigh risk vs. time to develop.

In this particular case you can just override the checkout message (admin/commerce/config/checkout/form/pane/checkout_completion_message) and set commerce_checkout_completion_message as a language dependent variable enables you to save a value per language.

Just add:

function mymodule_variable_info($options) {
  $variables['commerce_checkout_completion_message'] = array(
    'title' => 'Commerce checkout completion message',
  );
  return $variables;
}

to mymodule.variable.inc and then enable te variable at admin/config/regional/i18n/variable
and translate it at admin/config/system/variable/edit/commerce_checkout_completion_message

#1261012: Add Variable module integration for custom checkout completion messages

Marked #1475132: Default complete message use html links in t function as duplicate.

I patched #21 – it works like a charm!
Now I can translate those two strings at /admin/config/regional/translate/translate.

(refrehsing of all strings and clearing cache didn't worked to get the string in the translation interface; I had do create a new order, so that the string got displayed … after that it appeared in the translation interface)

Thank you, kotnik :)

Actually, the problem here is that this takes you back to storing actual URLs in the message once you provide an alternate checkout message. The purpose of the tokens is to avoid saving hard URLs so you can switch environments without invalidating URLs stored throughout the variable system like this.

+1 for #21 patch, i hope you apply this path in upcoming commerce 1.4

+1 for patch #21.

Ryan the patch don't save the URLs hard, it uses tokens just as the original code.

Or, am I misunderstanding you?

Ahh, you know what, I may have misread the patch and how it was using token_replace(). Marking this up for review again, though I can't promise the replacement tokens in the t() string will stay the same for folks who have already used this.

Edit: Patch towards 1.3. Ignore this.

I separated message in 3 t() strings. This is my code:

 function commerce_checkout_completion_message_default() {
   if (filter_format_load('filtered_html')) {
    return t('Your order is number [commerce-order:order-number]. You can <a href="[commerce-order:url]">view your order</a> on your account page when logged in.')
      . "\n\n" . t('<a href="[site:url]">Return to the front page.</a>');
   }

file commerce_checkout.module. Line 890

The idea of the patch in #30 works quite well, although I propose to use the tokens @order-number, @order-url and @site-url since these translations might already be in place.

...
You cant put html tags while translate strings via locale module.

Need something like this:

'Your order is number @ordernumber. You can @view_link on your account page when logged in.'
'view your order'

Two strings instead one!

Isn't it better to do just:

l(t('Return to the front page.'), '....')

Rerolled patch in #21 since it wasn't applying anymore. The patch looks good to me, here is what I've tested:
- Translated the strings on a local environment ( localhost/my_shop )
- Pushed the website to a staging environment (example.com/dev/my_shop)
- Checked that the text was translated and the url are correct

What else is missing to have this patch committed?

return t('Your order is number @ordernumber. You can <a href="@orderurl">view your order</a> on your account page when logged in.', array('@ordernumber' => token_replace('[commerce-order:order-number]'), '@orderurl' => token_replace('[commerce-order:url]')))
+      . "\n\n" . t('<a href="@fronturl">Return to the front page.</a>', array('@fronturl' => token_replace('[site:url]')));

No, you can't put html tags in t() function directly! Because locale module just can't validate string with html when you add translation in UI.

Full link tag must be included via "!placeholder".
Like this:

t('Your order is number @ordernumber. You can !view on your account page when logged in.',
  array(
    '@ordernumber' => token_replace('[commerce-order:order-number]'),
    '!view' => l(t('view your order'), token_replace('[commerce-order:url]'))
  )
)

As the message customization.

I think the best way is provide UI for users to change $string and $args arguments of t function:
t($string, array $args = array(), array $options = array())

$string as simple textbox and $args as some constructor for providing any placeholders based on tokens!

If DC commiters agree this (or similar) solution I can start working on this!

The workflow can be:

1. User put english string with the tokens(no html). (raw string)
2. Validator detect and check all tokens.
3. Module collect used tokens.
3.1 assign for each token placeholder
3.2 build own $args array
3.3 replace all tokens in string with placeholders (prepared string that should be used in t()).
4. Now we have customized $string and $args
5. User translate his string via locale module.
6. Show message
6.1 load prepared $string by variable_get
6.2 load prepared $args same way
6.3 do token replace
6.4 do t() call

Notes:

• User can see only bold steps (1,5,6).
• steps 3.1 - 3.3 can be moved to 6.* steps, can be cached or moved to separate function for use in 3.* and 6.*.
• now only links is a little problem, i think the better way is providing predefined tokens.

As for me, this solution seems so flexible and can be used not only for this issue!
Better solution is only integration local and token modules(can be done same way)!

Hm, as is see html tags is really not a broblem, so only tokens is a really problem!
http://drupal.org/node/322732
http://drupal.org/node/322774
Today I will do closer look to the sources...

@5n00py: Thanks for the review. The token issue has been slightly discussed in #11 and following. The problem with the previous token is that it was putting a possibly dangerous ":" inside html attributes. The patch I've rerolled in #36 should work fine (at least it does on my tests but maybe I've missed something).

@idflood Yes, now I see!
Sorry for blocking your status!
This is good patch as I see!
Maybe I can provide more flexible addition for your patch...
Just waiting for reviews!

Ok, take a look and test my patch!
Now we have full token support in checkout complete message!
The idea is described in #39 (some modified).

For example i have this message:

Dear [commerce-order:owner].
Your order is number [commerce-order:order-number]. You can < a href="[commerce-order:url]">view your order on your account page when logged in.

< a href="[site:url]">Return to the front page.

But string that go to t() function looks:

Dear @commerce-order_owner.
Your order is number @commerce-order_order-number. You can < a href="@commerce-order_url">view your order on your account page when logged in.

< a href="@site_url">Return to the front page.

All used tokens automatically goes to t() argument!

And now commerce_checkout_completion_message_default() should return untranslated string!

Fixed coding standarts!

Sorry, I can't go with this - removing the message string literals from t() makes them invisible to the translation string extractor.

Of course @rszrama!

I'll make some trick with this.
Literal contain @placeholders, but return of commerce_checkout_completion_message_default() always return string with [token:placeholders].

And if you don't need translated version just add options array to argument. Tokens saved after this. (Useful for settings form).

Also I found and fix a little related bug. "\n" generates wrong string (including \n signs itself).
"\r\n" fixed this problem.

#46: token_locale-1196488-46.patch queued for re-testing.
Testbot fail.

Thanks @5n00py, patch worked for me.

#46 The patch cannot be applied in the selected context

Let's up this one, @rszrama are you ok with the new approach?

Thanks for the patch, @5n00py! Gave this a review and test, here's some feedback. @5n00py, let me know and I can roll a new patch with some of these changes.

+++ b/modules/checkout/commerce_checkout.moduleundefined
@@ -887,12 +887,15 @@ function commerce_checkout_create_account($name, $mail, $pass, $status, $notify
+  //Provide tokens, they will be replaced after translation.

Inline comments need a space between the // and the comment. There are a few more of these in the patch.

+++ b/modules/checkout/includes/commerce_checkout.checkout_pane.incundefined
@@ -94,6 +94,7 @@ function commerce_checkout_completion_message_pane_settings_form($checkout_pane)
+    '#description' => t('Put English version here. Later You can translate it via Locale module'),

I think this description should just be removed, if anything the "Override the default checkout completion message" checkbox should have its label updated to indicate that it's translatable.

+++ b/modules/checkout/includes/commerce_checkout.checkout_pane.incundefined
@@ -139,8 +140,22 @@ function commerce_checkout_completion_message_pane_checkout_form($form, &$form_s
+  $message = t($message, $args);

This means that if you're using the default message it will go through t() twice since commerce_checkout_completion_message_default() returns a string wrapped in t().

One bug I found: if you're using a custom completion message, you will end up with two strings for translation (default and custom) - this is confusing. This is because commerce_checkout_completion_message_pane_checkout_form() always calls commerce_checkout_completion_message_default(). I would suggest changing the if statement to something like this:

@@ -123,8 +123,8 @@ function commerce_checkout_completion_message_pane_checkout_form($form, &$form_s
   $pane_form = array();
 
   // Load the appropriate default completion message.
-  if (variable_get('commerce_checkout_completion_message_override', FALSE)) {
-    $data = variable_get('commerce_checkout_completion_message', commerce_checkout_completion_message_default());
+  if (variable_get('commerce_checkout_completion_message_override', FALSE) && variable_get('commerce_checkout_completion_message')) {
+    $data = variable_get('commerce_checkout_completion_message');
 
     $message = $data['value'];
     $text_format = $data['format'];

This is the solution I ended up using, so here's an updated patch and interdiff to address the above, the only thing it doesn't address is:

This means that if you're using the default message it will go through t() twice since commerce_checkout_completion_message_default() returns a string wrapped in t().

# 53 works as expected, thanks

I don't understand the custom token replacement code in the bottom hunk; also, any reason to not set the $options array to the default parameter of the function?

Token replacement need for converting tokens(with symbols that's can't be translated, e.g. validation error) to @inline_arguments for t() function.

So in the message edition form you see [tokens], but in locale module you see @variables.

This patch use 'string literal' in t() call (for translation parsers), and one t() call with $variable (for token and custom message issue).

#53: commerce-1196488-53.patch queued for re-testing.

Just FYI. In #1702566: Redirect on checkout complete without using rules module. other approaches are proposed. Instead of providing a translatable message just load the content of a node and show it on the completion page.

any news on this? :)

If there was, the issue would be updated. On another look, I see that this patch continues to pass variables to t(), which is an improper use of that function.

@rszrama
Can it be solved using i18n_string/i18n_string_text or not ?
http://drupal.org/node/1114010
If yes I can start work on this?

Or we need to make some custom stuff which interact with core language system?
Give me direction to work & I will do this job.

If you want to use a completely custom thing, you can create your own simple pane real easy:

<?php
/**
 * Implements hook_commerce_checkout_pane_info().
 */
function mymodule_commerce_checkout_pane_info() {
  $panes = array();
  $panes['mymodule_completion_message'] = array(
    'title' => t('My custom message'),
    'page' => 'complete',
    'weight' => 15,
    'base' => 'mymodule',
  );
  return $panes;
};

/**
 * Implements base_checkout_form()
 */
function mymodule_checkout_form($form, $form_state, $checkout_pane, $order) {
    $markup = t("This is my new fabulous checkout complete message!");
    $form = array();
    $form['mymodule_complete_msg'] = array(
      '#markup' => $markup,
    );
    return $form;
  }
};?>

Then just disable the standard completion message.

We need to start from scratch here :)

The previous patch fixes translation of the default completion message by introducing a custom token format, and accompanying code.
This is not optimal for obvious reasons.
It also doesn't do anything about translation of overridden completion messages (the ones stored in the variable), t() can't be used there, we need i18n_string.

If we use i18n_strings, then the error is gone, and there's no need for a custom token format.
So, the solution is obvious: require i18n_string for any kind of completion message translation (default and overriden).
i18n modules (i18n_field and i18n_string, to be exact) are required anyway to translate some of Commerce strings (and D7 strings in general), and a non-english or multilingual site can't avoid it (they can just kick the ball further down the road with ugly hacks).

This patch:
1) Introduces a default Commerce textgroup.
2) Introduces a commerce_i18n_string() function that provides a fallback in case i18n_string is missing, and makes sure appropriate sanitization is done.
3) Implements translation of the checkout completion message.
4) Changes the format of commerce_checkout_completion_message_default(), so that this:

      $message = variable_get('commerce_checkout_completion_message', commerce_checkout_completion_message_default());

doesn't cause explosions due to mismatches in formats between the variable and the default.

Forgot to include the changes to commerce_checkout_completion_message_default().
This one is complete.

EDIT: Found a typo and a bug. New patch to follow.

This one should be committable.
The sanitization needs to happen after the token replacement, reverted that reordering and the translated text now shows properly.

EDIT: Small nitpick for before committing:

+  // Perform translation and apply the proper text format.

should be

+  // Perform translation.

since the text formatting is now applied a few lines down.

Looks good, I want to test it. :)

Reads fine to me and worked as normal in a non-translated interface. We'd need to note the API change for commerce_checkout_completion_message_default() on the off chance anyone was using it for something else. One quick regression question, though... by removing the default string from t(), are we not removing the ability for this string to be translated outside of the i18n module? As is, the string could be translated as normal so long as a custom message was not entered.

Patch attached with the updated comment.

Yes, with the latest patch, the default message is definitely not translatable without i18n_string.
Well, it wasn't before either (because of the triggered validation error, since t() can't handle embedded tokens), but we pretended.

Yeah, I was thinking that as I made the comment, but basically we have to decide either to fix it (there's an issue for it) or just remove the possibility. Is there any problem with leaving it in t() and fixing the token issue? Or perhaps we can just remove the whole checkbox thing for overriding the default message. Since we have a way to translate the text via i18n_string, we could get rid of t() and the extra configuration step altogether.

t() function can't validate '[]' symbols. For pass tokens to t() we can replace them with placeholders.

  $text_tokens = token_scan($message);
  $replacements = array();
  $args = array();
// Collect placeholders.
  foreach ($text_tokens as $type => $tokens) {
    foreach ($tokens as $token => $token_string) {
      $placeholder = "@" . "$type" . "_" . "$token";
      $replacements[$placeholder] = $token_string;
      // Perform token replacement.
      $args[$placeholder] = token_replace($token_string, array('commerce-order' => $order), array('clear' => TRUE));
    }
  }

$args array can used as t() argument .

I don't know how to use it properly, but maybe it is useful.

Also I think before string customized it just an interface string, that should be translated via t(). And token not required to place order_number in the message.

I definitely don't like the idea of inventing a custom token format & related code just to support t(), when we need i18n for other Commerce strings anyway.

I like the idea of removing the override checkbox. Patch attached.

Updated docs for commerce_i18n_string, to match the style I am adding in #1121722: Improve DX of instance translation.

We should also replace function_exists('i18n_string') with module_exists('i18n_string'), for consistency.
Forgot to include that in the patch, can be done pre-commit.

#73: 1196488-73.checkout_completion_message.patch queued for re-testing.

Just tried to test this patch but don't see the checkout completion string showing up in the translate interface even though I get the group. Also, thinking afresh about the no default translatable string, I can go either way: while we do need i18n for full field translation on a multilingual site, do we need it for a single language non-English site?

Looks like patch fully depends on i18n.

function commerce_checkout_completion_message_default() {
   if (filter_format_load('filtered_html')) {
-    return t('Your order is number [commerce-order:order-number]. You can <a href="[commerce-order:url]">view your order</a> on your account page when logged in.')
-      . "\n\n" . t('<a href="[site:url]">Return to the front page.</a>');
+    $value = 'Your order is number [commerce-order:order-number]. You can <a href="[commerce-order:url]">view your order</a> on your account page when logged in.'
+      . "\n\n" . '<a href="[site:url]">Return to the front page.</a>';
+    $format = 'filtered_html';
   }
   else {
-    return t('Your order is number [commerce-order:order-number]. You can view your order on your account page when logged in.');
+    $value = 'Your order is number [commerce-order:order-number]. You can view your order on your account page when logged in.';
+    $format = filter_fallback_format();
   }
+
+  return array('value' => $value, 'format' => $format);
 }

There is no t() call.

As I say before, not-overriden string should be passed through t() because its just an interface string.

while we do need i18n for full field translation on a multilingual site, do we need it for a single language non-English site?

Yes.
If you're doing any kind of translation, you need i18n.
As for the string not showing up, did you go to admin/config/regional/translate/i18n_string and click "Refresh strings"?

#77
This message can be user provided, and can contain HTML, therefore, it will never be wrapped in t().
Plus, adding t() would mean translating it twice when i18n is there.

Ok, I think the best way to summarize it is to say "Interface text translation is all managed through i18n." The checkout completion message as is tried to have it both ways, but it's best to just identify it as interface text that must be translated through i18n along with field instance labels, taxonomy terms, menu items, etc.

Committed w/ the change to module_exists().

Oh, and yeah, I had refreshed strings, but I didn't realize I had to check a box to refresh a particular group. : P

Commit: http://drupalcode.org/project/commerce.git/commitdiff/30f4a8e

@rszrama. Ryan, related to your comment #76, maybe you just need to refresh strings of the Drupal Commerce text group ?q=admin/config/regional/translate/i18n_string

I hate to be "that guy" but couldn't get those strings translated.

So, what was the outcome exactly? the 1.8 version should've had it fixed? Is it a bug at i18n? or else?

I made sure the latest version of Drupal Commerce was on, I tried even the latest Dev version, cleared caches and ran cron.

Please don't reopen old issues. You are free to open new support requests.

If you don't see the string on admin/config/regional/translate/translate then you must refresh the strings on admin/config/regional/translate/i18n_string.

I do see it there but get the error message saying that HTML is not valid.

When you don't need to use i18n you can do next workaround:

function MYMODULE_update_7001() {
  $message = commerce_checkout_completion_message_default();
  $message['value'] = t($message['value']);
  variable_set('commerce_checkout_completion_message', $message);
}

@a.milkovsky (comment #85): using your tip, the string is exposed to translation but can't be translated because of invalid HTML tags (even by pasting the original English text).

Do you mean message is not translated when it contains HTML?

Yes,
using Commerce 7.x-1.11 (and filtered HTML text format active), even the original string copied in translation textarea is refused:
Your order number is [commerce-order:order-number]. You can <a href="[commerce-order:url]">view your order</a> on your account page when logged in. <a href="[site:url]">Return to the front page.</a>

but is accepted without HTML tags:
Your order number is [commerce-order:order-number]. You can view your order on your account page when logged in.

Thanks

Comment #81 Is actually quite important, strange the variable isn't automatically indexed.

Sorry for reviving this old post.

I'd like to thank BramDriesen for pointing out comment #81.

This did the trick for us, when nothing else would make the string show up in the translation table. Strange indeed.