This patch fixes a context sensitive translation issue together with a ultra minor code style bug.

We could theoretically backport this to D6, but it's not important to do so and break an already translated D6 string.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ZoeN’s picture

Can you describe the actions I'd need to take in order to see the context-sensitive translation bug? (Or post a screenshot of the bug in action?)

Damien Tournoud’s picture

Status: Needs review » Needs work

!description should be @description. $description is plain-text, so we need to escape it before output in an HTML context (there is no security issue here).

hass’s picture

Status: Needs work » Needs review

Damien: no, see a few lines earlier (not visible in the patch)... the value need to be added as is. !description is correct

Damien Tournoud’s picture

Status: Needs review » Needs work

I'm not sure what you mean by "the value needs to be added as is". What I see is:

 * @param $description
 *   A plain English description of what this hook operation does.

... which suggest that the description is plain-text... and everywhere else, values from the $description array used encoded:

t('Are you sure you want to unassign the action %title?', array('%title' => $actions[$action]['description'])),
watchdog('actions', 'Action %action has been unassigned.',  array('%action' => check_plain($actions[$aid]['description'])));
drupal_set_message(t('Action %action has been unassigned.', array('%action' => $actions[$aid]['description'])));
hass’s picture

Today it is added as is and this shouldn't change... maybe a module action likes to add HTML code as description. Maybe only a <em>, <strong> or <code> to highlight something in the description. This is possible today as I know...

andypost’s picture

Status: Needs work » Needs review
FileSize
648 bytes

Damien you are wrong, take a look at http://api.drupal.org/api/function/hook_trigger_info/7 - labels already passed through t()

So here a re-roll

hass’s picture

Then you can use my patch as it also fix a minor code style bug :-).

andypost’s picture

FileSize
759 bytes

@hass yes, your patch was right but it no longer applies so here re-roll with code-style fix (comma at the end :)
You can RTBC ? or we need someone to review?

hass’s picture

Status: Needs review » Reviewed & tested by the community

This trivial patch looks codewise RTBC.

Damien Tournoud’s picture

Acked. If it is passed through t(), it's HTML.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Looks good. Committed to HEAD with the addition of an inline comment:

    // !description is correct, since these labels are passed through t() in
    // hook_trigger_info().

I'm sure that myself and Damien are not going to be the last people who look at this and believe it's a mistake. ;)

Since this is going to break strings in D6 for no real gain (echoed by hass in the original issue), I'm marking fixed rather than 6.x.

Status: Fixed » Closed (fixed)

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