Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Since locale's t() function relies on database storage we have a special function for translating during the installation, updates, etc. That is http://api.drupal.org/api/drupal/core!includes!install.inc/function/st/8
1. The first problem is: when a exception is thrown, we use low level exception handling in error.inc for it but our exception handling relies on t() for translating messages, which in turn relies on database access.
Therefore, the install system cannot even properly handle exceptions (because exception handling relies on db access)
This patch, first step, removes all t() from exception handling functions (error.inc).
2. Many exceptions are thrown with a localized message, thus they won't work from the installer either. I think ideally all our exceptions should be not localized unless we are pretty sure they will be thrown only when the full system is initialized. Or maybe better, since that's difficult to know, we throw only English exceptions.
Example: (From Drupal\Core\Database\Query\Merge.php, line 411)
throw new InvalidMergeQueryException(t('Invalid merge query: no conditions'));
Which is so cool, our Database exceptions depend on the database to be properly handled :-( . There are 79!!! more examples of that in Drupal core.
About this second problem, see #817160: Don't translate exceptions
Related issues
#817160: Don't translate exceptions
#1802278: Add a Date component to core
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 1808864-8.patch | 2.54 KB | fastangel |
| #6 | 1808864-6.patch | 2.53 KB | fastangel |
| locale_safe_error_handling.patch | 2.14 KB | Jose Reyero | |
Comments
Comment #1
Gábor HojtsyIt would be important to mark these with some comment to note why are those not using t() so someone does not come around later to add t() to them considering the missing t() a bug.
Comment #2
andypostAnother way - to add special context for exceptions and messages. This could be very helpful for translation
Core is growing with files that holds just a simple named exceptions inside - this could be unified after feature freeze
Another topic to discuss - how to deal with exceptions that thrown by PHP itself. They are displayed with server's locale so some code should avoid to use messages for errors (see first hunk in #1802278-42: Add a Date component to core )
Comment #3
andypostYes they should be untranslated according to #817160: Don't translate exceptions
Comment #4
Jose Reyero CreditAttribution: Jose Reyero commentedOk, thanks @andypost, so this issue should be only about exception handling (remove t from 'error.inc'). I'll update the description to point to the other issue about removing t() from inside exceptions / exception throwing.
Comment #4.0
Jose Reyero CreditAttribution: Jose Reyero commentedAdded related issues section
Comment #5
Gábor HojtsyOnce again, I think the changes look good, but commenting them so people don't add back the removed t()-es later would be important. I'd imagine adding a comment like this would be useful to each change:
Comment #6
fastangel CreditAttribution: fastangel commentedAttached new patch with the comments. Is need other thing in this issue?
Comment #7
Gábor HojtsyThanks! Should have a space after the // and then it should be good IMHO :)
Comment #8
fastangel CreditAttribution: fastangel commentedNow yes.
Comment #9
Gábor HojtsyLooks good :) note that this is about exception handling, the exceptions themselves are at #817160: Don't translate exceptions.
Comment #10
webchickOk, makes sense.
Committed and pushed to 8.x. Thanks!
Comment #11
Gábor HojtsyWoot, thanks all! Let's handle exceptions themselves in #817160: Don't translate exceptions.
Comment #12.0
(not verified) CreditAttribution: commentedUpdated issue summary.