Posted by catch on June 3, 2010 at 2:28pm
10 followers
Jump to:
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
Issue Summary
#816642: Two improved field exceptions pointed out that we have a mixture of translated an untranslated exceptions in core. I agree they shouldn't be translated, but t() placeholders are handy. I vaguely remember we had a non-translating function which did basic placeholder replacement but can't remember what or where it is, having to do sprintf() every time feels like a pain, putting variables directly into the string encourages bad habits for XSS even if the individual cases are safe. Either way, opening an issue.
Comments
#1
http://api.drupal.org/api/function/_drupal_render_exception_safe/7
#2
Are there any cases, where this would be unsafe? Otherwise, I think it should be fine to do that as a standard. We already do that in SimpleTests also.
#3
We discussed this today on irc://irc.freenode.net/drupal-i18n and +1 on not using t() inside exceptions.
#4
Regarding variables in exception strings: I personally still don't find anything wrong with simple string concatenation, but in tests we're more or less consistently using the (semi) new format_string().
#5
There's a follow-up #1808864: Exception handling is not 'locale safe' (thus not database safe)
and #1802278-42: Add a Date component to core
#6
Indeed, format_string() :)
#7
So maybe we should add a better description for this issue and work on a patch for removing t() out of all exceptions?
- About why translating exceptions is actually a bug, it is explained here #1808864: Exception handling is not 'locale safe' (thus not database safe)
- The replacement function for 't()' in this case is 'format_string()', @Gábor #6
However I think we should push this patch here before, because there's no point in making exceptions not using locale when the exception handling code is using it heavily: #1808864: Exception handling is not 'locale safe' (thus not database safe)
#8
I often wondered whether it is a good idea to translate these and I had no idea why did we attempt: these are not normal user facing strings and I am sure it can cause serious trouble calling such a complicated API as t()/locale() when you are on your way out because of the error handling.
Exceptions (heh) need to be made for user facing exceptions, at least field errors are such.
#9
Symfony 2 uses sprintf without any issues.
When you throw exceptions, you generally knows where your data come from, and using sprintf does seem as bad.
Anyway, I would not use t() at all, exceptions should be in plain english, as the documentation is. Exceptions are for programmers, not users.
Error messages, instead, are generated when catching an exception; they should be translated, because the user will get them displayed.
#10
Suppose core should use php function
setlocale()to try to init language 'EN' for messages