String translation cannot imported, invalid string in t()

Hi tobiasb,

Thank you very much for your interest and help reporting issues, it is highly appreciated.
Really sorry for this late reply, but I have only started really digging into the pending issues in module's tracker.

Alright, so I have taken a quick look at the submitted patch from #1 and I'm not exactly sure what this patch would be adding:

- Could you please elaborate with a little bit more details what you would actually like to achieve here?
- Could you please let me know what this patch really changes for you?

Wouldn't it be possible to translate the strings:

• <mail id> is missing
• <to> is missing

with the current module already?

Looking at reroute_email.module, lines 116 and 117:

<?php
     $mailkey = isset($message['id']) ? $message['id'] : t('<mail id> is missing');
     $to = isset($message['to']) ? $message['to'] : t('<to> is missing');
?>

These text strings seem to be correctly wrapped in a t() function calls that should allow translation.

Did you want to move out <mail id> and <to> from these strings for translation because the string localization doesn't allow any kind of special markup or something like that?
[I think there is an option/configuration in the Locale module to allow specific input formats or HTML for the translation of these strings - to be further confirmed because I'm not sure whether this feature would be provided by the Locale or I18N module.]

Currently, I'm not exactly sure why this patch would be needed, so I would greatly appreciate if you could explain with a little bit more details so that I could carry out further tests.
So I'm marking this ticket as needs work until I am able to get some more information from anyone on this particular patch and request.
Please change the status back to needs review if you are able to provide more information and answers to these questions.

Feel free to let me know if you would have any questions, comments, suggestions, recommendations or feedback on any points mentioned in this comment, I would certainly be glad to provide more information.

Thanks in advance for your replies, comments and feedback.
Cheers!

taking this task, will improve a little :)

tobiasb is absolutely correct:
Drupal does not allow to import strings which are wrapped with <> and are not validated html.
e.g.

t('<wrong>');

is not a valid string for t() function, as it contains '<' and '>'. Instead, tobiasb offers to carry these 'prohibited characters' out into a placeholder, and that's absolutely good;

but, I thought that 'mail id' and 'to' words should be translated as well (e.g. 'ИД письма' and 'кому' in Russian), so I wrapped them into t() as well.

please, consider adding the patch, which repeats tobiasb's work and adds my own small improvement.

p.s. this is my very first patch to Drupal community, so if I have any errors made (not in code, but in creating the patch :) - I do have some experience in developing modules for Drupal), you are appreciated to point them out to me

Hi yaryar and tobiasb,

Thanks very much for your patches and follow ups.

So I took some more time to look at this issue in detail and actually this is what I think:
The problematic strings in question in this ticket are (as indicated in the issue summary):

• <mail id> is missing
• <to> is missing

These strings actually seem to refer to the two array keys $message['id'] and $message['to'], of the $message array as a parameter of the implementation of hook_mail_alter.

Unless these strings (<mail id> and <to>) have a particular use/logic in:
<?php $message['headers']['X-Rerouted-Mail-Key'] = $mailkey; ?>, in reroute_email.module, line 119
and <?php $message['headers']['X-Rerouted-Original-To'] = $to; ?>, in reroute_email.module, line 139

if I understand correctly, these don't seem to have any use other than telling/indicating to developers that these keys are missing from the $message parameter passed to reroute_email_mail_alter:
see reroute_email.module, lines 146, 147

<?php
        $msg .= t("Mail key: @key", array('@key' => $mailkey)) . "\n";
        $msg .= t("Originally to: @to", array('@to' => $to)) . "\n";
?>

 
Since I couldn't find any particular programming logic or specific code related with any of these "tokens", please correct me if I'm wrong, but I wouldn't really see any reason why they couldn't/shouldn't be changed to:

• [mail id] is missing
• [to] is missing

in which case the problem related with <> characters would simply disappear, while altering/deteriorating very little (actually, nothing, in my opinion, if there is no side-effects on Email Headers) from the functions or meaning of the message sent to developers (or the ones to receive rerouted emails).

Therefore, I would like to get your opinion/reviews/feedback on the following attached patch against reroute_email-7.x-1.x at e278a91, which simply changes 4 characters (< becomes [ and > becomes ]):
File named: reroute_email-invalid-string-in-t-1852810-6.patch

Could you please let me know if you would see any problem or side effect that could be caused by this change on these strings?
I tried to find more information on the email headers X-Rerouted-Mail-Key and X-Rerouted-Original-To but couldn't find any standard or RFC documents that would clearly specify against or anything on the syntax for strings.
Since, I'm not deeply familiar with Email Headers Standards and related RFC, I would really appreciate some feedback, testing and reporting on this.

Please let me know if you would have any questions, comments, issues, recommendations, objections or concerns on the attached patch or any of the aspects discussed in this comment, I would certainly be glad to provide more information, explain in more details or re-roll the patch if necessary.

Special thanks to yaryar for your enthusiasm, efforts and activity on this ticket, please keep up the good work!
(we greatly value your help and have plenty of work)

Any questions, feedback, testing, changes, recommendations would be highly appreciated.
Thanks to all in advance.
Cheers!

Additional Note:
The only place I found X-Rerouted-Mail-Key and X-Rerouted-Original-To referenced in the tracker is #1221524-7: option to turn off the prepended message and put it in e-mail headers instead and unfortunately, there is not much information about any potential usage.

Hi tobiasb,

Thanks very much for your prompt reply.

So basically, you would like to prevent users from translating "mail id" or "to", and I would be curious to know if there would be any reason for that?

Wouldn't something like t('[mail id] is missing') give users the possibility to translate or not this string?
Leaving the possibility to have: [French translations]

• [identifiant email] absent ([mail id] translated)
• [mail id] absent ([mail id] not translated, copy/pasted by user for example)

Once again, please correct me if I'm wrong, but I wouldn't see any reason why we should prevent users from doing what they want with these strings.

Especially, if you look at #5, it seems that yaryar wanted to be able to potentially translate these (.... why not?!):

I thought that 'mail id' and 'to' words should be translated as well (e.g. 'ИД письма' and 'кому' in Russian), so I wrapped them into t() as well.

Could you please let me know if you would see any particular problem that could arise from the translation of any of these strings?

Please let me know if you would have any questions, further suggestions or concerns on this particular approach, I would certainly be glad to provide more information or explain in more details.

Once again, thanks to all in advance for your reviews, answers, testing and reporting.
Cheers!

Thanks very much tobiasb for your kind reply and comment.

If there is no particular "critical" objections, in the sense that it would cause a bug (with Email Headers, for example), then I would prefer keeping it as I suggested in #6, since I'd like to leave the choice to users to translate or not the full strings.

Let's see if we could hear anything from yaryar or any other peers about this.
I suggest to keep this issue open in status needs review and wait a little bit to see if we can get any more feedback, testing, comments and reviews.

Feel free to let me know if you would have any questions, further suggestions or concerns on the patch from #6 or any other aspects discussed in this ticket, I would certainly be glad to provide more information or explain in more details.

Once again, thanks to all in advance for your reviews, answers, testing and reporting.
Cheers!

Alright guys,

Two weeks have passed since the last update at #10 without hearing any further comments from @yaryar or any other users, on this issue.
So I went ahead and got this committed at:

• 7.x-1.x: 4930477
• 6.x-1.x: 8c5362b

I allowed myself to mark this issue as fixed for now, but feel free to re-open it, or post a new ticket, at any time if you have any further objections with the approach suggested at #6 (we would surely be happy to hear your feedback).

Please let me know if you would have any further comments, feedback, questions, issues, objections, suggestions or concerns on any of these commits or this ticket in general, I would be glad to provide more information or explain in more details.

Thanks again to everyone for your help, reviews, feedback and comments on this issue.
Cheers!