define(FOO, t("bar")) is a performance disaster.
robertDouglass - October 4, 2008 - 10:03
| Project: | Comment Mail |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
Any time you try to translate something before the bootstrap is finished it rebuilds the entire cache of localized strings. Thus, non-English sites with this module have to build the entire cache every page load resulting in an extra query for every string, every page load (yes, this is how I've seen sites have 14,000 queries, every page).
Can't roll a patch right now, but here is what needs to be changed:
<?php
diff -u -r1.15 commentmail.module
--- commentmail.module 22 Jun 2008 15:56:23 -0000 1.15
+++ commentmail.module 4 Oct 2008 10:02:15 -0000
@@ -6,7 +6,7 @@
* - integrate with token module
*/
-define('COMMENTMAIL_DEFAULT_APPROVE', t("An unapproved comment has been posted on @site for the post '@node'. You need to publish this comment before it will appear on your site.
+define('COMMENTMAIL_DEFAULT_APPROVE', "An unapproved comment has been posted on @site for the post '@node'. You need to publish this comment before it will appear on your site.
approve/edit/delete/ban: @quick_approve
@@ -19,9 +19,9 @@
approve/edit/delete/ban: @quick_approve
-Comment administration: @admin_url"));
+Comment administration: @admin_url");
-define('COMMENTMAIL_DEFAULT_NOTIFY', t("A new comment has been posted on @site for the post '@node'.
+define('COMMENTMAIL_DEFAULT_NOTIFY', "A new comment has been posted on @site for the post '@node'.
THIS COMMENT DOES NOT REQUIRE APPROVAL
@@ -34,7 +34,7 @@
edit/delete/ban: @quick_approve
-Comment administration: @admin_url"));
+Comment administration: @admin_url");
/**
?>Note that you already translate the constants later in the code, so the strings are being double translated.
#1
Here it is in a proper patch form. I'm using this on a site without issue. Is translation really needed for these messages? If so, perhaps the module should call t() while building the form, instead of in the define.
--Andrew
#2
Here's the link to the D7 core issue about this: 320686: Doc: don't use t() in global contexts.
#3
You've missed one of the two in the above patch. Attached is a patch with both constants fixed.
#4
Fixed! And my first commit for this module :)
Thanks robertDouglass and deviantintegral.
#5
Awesome, thanks for getting the one I missed and committing this.
#6
Automatically closed -- issue fixed for two weeks with no activity.