when generating the .pot file for project_issue using the php potx-cli, I had the following error:

[augustin@203 project_issue-5]$ php potx-cli.php
Processing comment.inc...
Processing issue.inc...
Invalid marker content in issue.inc:1600
* t($node->category)

Processing mail.inc...
Processing project_issue.module...
Processing project_issue.install...
Invalid marker content in project_issue.install:748
* t("This and all subsequent updates of Project issue were safely aborted. Correct the problems listed above, then <a href=\"".base_path()."update.php?op=selection\">re-run update.php</a>, click 'Select versions', select update %update for project_issue, and click 'Update'.",array('%update'=>$current_update))

Processing project_issue.info...
Processing generate/project_issue_generate.inc...
Processing generate/project_issue_generate.module...
Processing generate/project_issue_generate.info...

Done.

Dereck confirmed that it is project_issue bug, not potx. You can't put dynamic stuff inside t().

CommentFileSizeAuthor
#6 project_issue_category_fix.patch2.53 KBdww
#3 pot_fix.patch2.62 KBhunmonk
#2 stuff-inside-t-206502.patch2.58 KBbeginner

Comments

hunmonk’s picture

hunmonk commented
Status: Active » Postponed (maintainer needs more info)

the second one should be pretty easy to fix. for the first one, should we bag the t() call altogether?

beginner’s picture

beginner commented
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new stuff-inside-t-206502.patch2.58 KB

I removed the t(), but I'm not sure it's the right thing to do.

The second one was one loooooooong line, so I broke it into bits.

hunmonk’s picture

hunmonk commented
StatusFileSize
new pot_fix.patch2.62 KB

cleaned this up a bit, and added a check_plain() around $node->category, for future-proofing.

beginner’s picture

beginner commented
Status: Needs review » Reviewed & tested by the community

Your !update_path is clearer than my !base_path ! :)
I mean the code is clearer this way.

dww’s picture

dww
we/he/they
commented
Status: Reviewed & tested by the community » Needs work

Nope. ;) project_issue_category() already maps the internal, 1-word english versions into the t()'ed, "human readable" versions of each category. We should always be using project_issue_category($node->category) whenever we publicly print a category somewhere. The check_plain() isn't needed, since it's always hard-coded, though, i suppose for the utterly paranoid, it doesn't hurt. However, we should really make sure we use project_issue_category() correctly when printing these out, so that we always get the right thing (even for d.o and english-only sites).

Also, I thought we always wanted check_plain() on URLs inside t() calls, so it should be @update_path not !update_path for the hunk against project_issue.install.

dww’s picture

dww
we/he/they
commented
Title: potx error: dynamic stuff inside t() » improper or missing uses of project_issue_category() causing translation bugs
Status: Needs work » Needs review
StatusFileSize
new project_issue_category_fix.patch2.53 KB

I tested the change in project_issue.install from the previous patch -- that was good, so I committed to HEAD.

Converting this issue to deal with improper/missing uses of project_issue_category(), as per my previous comment. Here's a (moderately tested) patch. Another pair of eyes wouldn't hurt, but I'm pretty sure this is what we want. As a separate patch like this, it should also hopefully be easier to backport, since the project_issue.install hunk is specific to HEAD (and lo, I just tried and the patch applies (with minor fuzz) on all branches -- yay!).

oadaeh’s picture

oadaeh commented
Status: Needs review » Reviewed & tested by the community

Looks good to me and worked correctly for me.

dww’s picture

dww
we/he/they
commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD, DRUPAL-5, DRUPAL-4-7--2 and DRUPAL-4-7.

beginner’s picture

beginner commented

thanks.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)

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