Wrong usage of t() : Database bloats over time
| Project: | SEO Checklist |
| Version: | 5.x-1.3-1 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Hi,
thank you for the nice module! Yesterday I had a look at the .po translation file of my drupal installation and unfortunately noticed a bunch of very large entries generated by the SEO checklist module.
The seo checklist module uses the t() not in the way it is meant to be: Mainly, you should never wrap whole pages in t() (see line 227). And if these pages have dynamic content, you will have another unpleasant effect: On every change of that page, the whole page (!) with each and every html element gets stored as a new entry in the locales tables... And the database bloats over time.
BTW: I also had problems with t() until Derek explained it to me (http://drupal.org/node/350133#comment-1167684). So, welcome to the club ! ;o)
Thanks,
Oli
#1
Awesome! Thanks for the details. My gut tells me to change this:
'#value' => t($page_contents)
to this:
'#value' => $page_contents
Would that fix the problem or make it worse?
I've moved on to supporting the version 6 compatible of this module and it has the same issue. I'm a non-programmer poking my way through this so any help is GREATLY appreciated.
--Ben
#2
Hi Ben,
yes, this would fix the major problem. But then the most part of the modules interface won't get translated anymore.
I would like to suggest another fix: Today I spent some time on going through the module code in order to adapt the coding style to the drupal way. This helps to keep the code more easy to understand and to use the translation function t() for the necessary parts. Additionally, drupal provides some quite useful ways to handle database requests in a secure manner.
I put some notes inside the code that could be of help for your programmers.
The main changes:
- put all the styling in a separate css file
- further styling/ theming will be provided by a separate theming function
- t() is only used for the relevant text strings
- changed most of the database calls to be conform to the drupal form api way (not for all, although most of the database calls are already changed. But it shouldn't be a problem for your guys to finish it, I think.)
Hope it helps.
Best wishes,
Oli
#3
Try 2.0. Download it here: Drupal SEO Checklist