Posted by brianV on January 8, 2010 at 8:33pm
6 followers
| Project: | Job queue |
| Version: | 6.x-3.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
Issue Summary
<?php
$rows[] = array(
format_date($job->created),
t($job->description, array_filter(unserialize($job->arguments), 'is_scalar')),
);
?><?php
watchdog('job_queue', 'Ran queued job "!description"', array('!description' => t($job->description, array_filter($arguments, 'is_scalar'))));
?>Why are we sending the list of function arguments on to t()? This is resulting in output as per the attached screenshot.
| Attachment | Size |
|---|---|
| Screenshot-7.png | 26.25 KB |
Comments
#1
Oh yes. I just stumbled over that too.
Subscribing :-)
#2
This was causing me headaches, too. The arguments are indexed with numeric values (0, 1, 2, etc), so if you have any numbers in your description, they will be output. Very "wonky" indeed. I recommend we remove it. The attached patch does so.
#3
I'd argue that the description shouldn't be translated at all in the job queue module. You should translate it yourself when calling job_queue_add().
Tweaking patch #2 slightly.
#4
Revised patch handles watchdog() as well
#5
c4rl: Nice patch. However, I do think the watchdog call with the next 'Ran queued job' should run through t() as that is english text.
#6
I see what you mean. I have a few remarks and realizations which may influence a different kind of patch.
First of all, in dblog.module, watchdog messages are automatically sent to t() (see dblog_overview() and _dblog_format_message()), so I believe the English text should be fine.
At first it made sense to me that the usage of the function should send the translated string as an argument of job_queue_add().
For example:
<?php// Description is second argument -- send translated string.
job_queue_add('drupal_mail', t('Sending !key mail to @address', array('!key' => $key, '@address' => $address)), array('my_module', $key, $address, language_default(), array('foo' => $foo)));
?>
In this fashion, job_queue only has to deal with the translated descriptions. This brings up a problem, however, that the descriptions are then not translatable -- a multilingual admin interface isn't possible.
Arguably (no pun intended), there should be a separate parameter for job_queue_add() just for the parameters to be translated. I suppose in order to *not* break the API outright, we could accept the $description argument as either (1) an untranslated string or (2) an array of arguments to be used in t().
If this makes sense, I can probably roll the patch later this week to demonstrate.
#7
any progress plz? bumping ...
#8
I agree with @c4rl in #6, please do roll a patch with that =)