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.This is a bad string for translators.
Also, at least in 7.x, calling l() within system_token_info() triggers a theme registry rebuild, which adds 11 seconds to a call to drupal_flush_all_caches() on the site I'm profiling, which is how I found this.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | drupal7_2339021_6_system_token_info.patch | 704 bytes | er.pushpinderrana |
| #2 | 2339021_system_token_info-D7-do-not-test.patch | 704 bytes | catch |
| #1 | 2339021_system_token_info.patch | 718 bytes | catch |
Comments
Comment #1
catchComment #2
catch7.x patch.
Comment #3
larowlanWow. 11 seconds. Just wow.
Comment #4
alexpottCommitted db253b0 and pushed to 8.0.x. Thanks!
Comment #6
er.pushpinderrana CreditAttribution: er.pushpinderrana commentedjust uploading #2 patch again for D7 to get pass through testbot.
Comment #7
tstoecklerActually the proper way to do this is
Let's not backport a suboptimal patch. Not touching the issue metadata because I always get yelled at when "re-opening" issues.
Comment #8
catch@tstoeckler see #2339219: [meta] Finalize URL generation API (naming, docs, deprecation) - we currently don't have a non-deprecated replacement for url() that handles this case. Also since the URL isn't user input url() wouldn't do anything here.
Comment #9
tstoecklerWell, I still think it's preferable for translators to just have '@url' instead of the actual URL in the string, as in the code example above. Not a biggie, either way though and the committed patch is certainly an improvement.
Comment #10
mgiffordLooks good, applies nicely, bot likes it, it's a simple patch, already in D8. Let's RTBC it....
Comment #11
catch@tstoeckler we could do !url to keep the placeholder substitution? Although also with that particular link it may be possible for translators to change the link to point to the correct language too?
Comment #12
tstoeckler@catch: Interesting, that's actually a fair point, that you'd want the link to change per language. So in that case, putting the URL into the translated text makes sense. In that case, however, we should strive to make the URL a little bit less painful to look at: http://php.net/en/date should be completely sufficient and allows translators to switch the language code transparently (instead of http://php.net/date which would work as well).
Moving back to D8 for that.
Comment #26
quietone CreditAttribution: quietone as a volunteer commentedThis issue was committed in 2014 and re-opened to make another improvement to the change made here. The Bugsmash initiative has found that progress is made by closing issues like this and opening a new issue to make the improvement. Therefor I am closing this issue and opening a new one to make for the change suggested in #11 and mark that one as needing backport. After all, it modifies the one line changed in the commit.
The followup is #3238194: Use !url placeholder in description string in system_token_info and added credit there.