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.
Token replacement, e.g. in the "Welcome" message sent during registration, is not working because the *.token.inc files are never included.
Comment | File | Size | Author |
---|---|---|---|
#49 | drupal.boot-docs.49.patch | 4.26 KB | sun |
#48 | 642782_better_language_init_docs.patch | 980 bytes | greggles |
#44 | language_init_bootstrap-642782-44.patch | 2.88 KB | plach |
#43 | language_init_bootstrap-642782-43.patch | 2.88 KB | plach |
#38 | language_init_bootstrap-642782-38.patch | 2.31 KB | plach |
Comments
Comment #1
mfbComment #2
Dave ReidComment #4
arhak CreditAttribution: arhak commentedregarding this issue, think about its "backport", which would consist only in some documentation #681938: document proper practice for mymodule.token.inc files
Comment #5
Dave ReidI can't duplicate this since it works just fine for me. This shouldn't be necessary since module_implements() should auto-include the .tokens.inc files. Please re-open if you can re-confirm with latest HEAD code.
Comment #6
mfbIt looks to me like I'm somehow getting an empty hook_info cache when bootstrap mode causes a module list without system module to be built (the token hook_info is provided by system module). I'll see if I can figure out how this is happening -- not sure if it's a particular configuration or something I broke on this site..
Comment #7
mfbOK I'm not crazy, you should be able to reproduce this if you enable a second language.
The empty hook_info array is getting cached during module_invoke_all('language_init', $types) in drupal_language_initialize().
Comment #8
mfbProbably an issue in language or base system.
Comment #9
Dave ReidCan you see if #710860: Static caching in drupal_multilingual() breaks tests fixes it?
Comment #10
Dave ReidOh wow, yeah you're right. Enabled a second language and *poof* there went tokens. So the problem is when we have multiple languages enabled, the call to module_invoke_all calls module_implements before we have a full bootstrap, add all modules loaded. So its caching the wrong information.
Comment #11
Dave ReidOk got this figured out. Since its called before we have a full bootstrap, we need to convert this to bootstrap_invoke_all() so that hook implementations aren't cached too early.
Tested the attached patch and confirmed that we get the correct results from module_hook_info() and tokens worked.
Comment #12
Dave ReidUpgrading to critical as it breaks functionality completely if more than one language is enabled.
Comment #13
mfbOK had to disable/enable a module to get the bootstrap_hooks() to refresh.
You changed the input for hook_language_init() implementations to a nested array.
Without patch:
Array ( [0] => language [1] => language_interface [2] => language_url )
With patch:
Array ( [1] => Array ( [0] => language [1] => language_interface [2] => language_url ) )
Comment #14
Dave ReidAck, yeah. Give this one a try. It uses the same format as module_invoke_all().
Comment #15
mfbIdeally we could avoid cufa() altogether (for speed), if we have a defined set of bootstrap hooks with at most one argument.
Comment #16
sunComment #17
mfbThis should invoke all rather than returning while iterating thru the modules.
Comment #18
Dave ReidFixed. Back to RTBC.
Comment #19
jurgenhaasI'd love to test and review this patch but it is outdated and can't be applied to the current D7 dev release. Could you please re-roll the patch?
Comment #20
Dave Reid#18: 642782-language-init-bootstrap-D7.patch queued for re-testing.
Comment #21
jurgenhaasThe patch is for bootstrap.inc revision 1.352 but the current file is of revision 1.362 and the file has changed a bit since.
Comment #22
gregglesThe patch still applies fine. Here it is re-rolled to remove some offsets.
Comment #23
sunSome other patch got mixed into that one.
Comment #24
gregglesThanks for catching that.
Comment #25
moshe weitzman CreditAttribution: moshe weitzman commentedcode looks good to me. nice detective work
Comment #26
jurgenhaasWorks fine here too.
Comment #27
Dries CreditAttribution: Dries commentedI'm confused. I searched core for language_init and didn't came up with any results.
drupal-cvs dries$ grep -r "language_init(" *
Comment #28
Dave ReidThere aren't any core implementations of it. It's merely for contrib I'm guessing. But the fact is it causes major problems with module_implements() caching.
Comment #29
mfb@Dries hook_language_init() is a hook that can be used by contrib modules; it's not used by core.
The problem is an empty hook_info array is getting cached during module_invoke_all('language_init', $types) because most modules are not yet loaded at this point in bootstrap. by calling bootstrap_invoke_all() instead, you avoid generating the cache.
Comment #30
Dries CreditAttribution: Dries commentedI'd like to better understand that hook then. Is there a reason that hook should be called language_init? Why can't those modules use _boot() ? It sounds to me like this warrants some code comments.
Comment #31
mfbThere's an open issue here #593746: Prepare Drupal core for dynamic data translation where this undocumented hook is supposed to be documented..
Comment #32
Dries CreditAttribution: Dries commentedI was asking about the name of the hook because other modules might want a similar hook? I'm not sure other use cases exist. If they are other use cases for a similar hook, we should probably go with a more generic name?
Comment #33
mfbI don't see any problem with the hook_language_init() name. Core has already staked a claim on hook_language_, see http://api.drupal.org/api/search/7/hook_language_ and the name generally makes sense given that it is invoked by drupal_language_initialize() during the DRUPAL_BOOTSTRAP_LANGUAGE phase of bootstrap.
Comment #34
plach@Dries:
hook_language_init()
was introduced to have a safe method to translate configuration variables, translation that cannot rely on the method proposed in #593746: Prepare Drupal core for dynamic data translation; IIRC the_init
suffix has been given because the hook invocation happens in the language initialization phase.To generalize this approach we might want to introduce a bootstrap hook to alter the configuration variables (e.g.
hook_configuration_alter()
) at a proper bootstrap phase. This must happen after language initialization to be useful, though.Comment #35
Dries CreditAttribution: Dries commentedThanks for the explanations. That seems to make sense. Could we, at a minimum, at some minimal documentation so we don't end up scratching our heads 2 years from now?
Comment #36
plachHere is the hook documentation. I reverted the cufa change as the
$types
parameter was uneeded.Comment #38
plachrerolled
Comment #39
sunLooks good, thanks!
Comment #40
Dries CreditAttribution: Dries commentedI'm still slightly puzzled for the need of this hook. Typically, variables are translated on output.
Comment #41
sun@Dries: Actually, no. :) Variables are user-defined strings and therefore shouldn't be translated via t() at all, since the source string can change without notice and you'll lose any translation you had for it. The i18n module for D6 implemented a tt() function to translate arbitrary user-defined strings, but that didn't gain full momentum throughout Drupal. A couple of Internationalization team members are now working on new translation systems for D7 over in the http://drupal.org/project/translation project. Among those is #593746: Prepare Drupal core for dynamic data translation, which identified the need for a dedicated hook to translate variables, which runs as early as possible in the bootstrap (ie. directly after variables and the language system have been initialized).
Comment #42
Dries CreditAttribution: Dries commentedThanks for the explanation in #41. I think we could make the PHPdoc of the example code in the patch a bit more meaty by embedding some of that information into it. It would have avoided my questions.
Comment #43
plachmeatier docs :)
Comment #44
plachtrailing whitespace :(
Powered by Dreditor.
Comment #45
sunComment #46
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #47
chx CreditAttribution: chx commentedthat's incorrect, either add it to hook_boot or change it to hook_language_init
Comment #48
gregglesSo, something more like this?
Comment #49
sunA good chance to clean a couple of things up.
Comment #50
plachLooks good. Just a question: why were the lines above removed?
Powered by Dreditor.
Comment #51
sunNo longer sure why I removed it. I guess because it sounds like we are suggesting that it is a good idea to globally and unconditionally add JS/CSS on all pages.
Comment #52
plachIMO the comment above suggests that might exist cases in which makes sense adding JS/CSS on all pages. What about restoring it? ;)
Comment #53
plachBugs are fixed in the development version first, backported then.
The patch needs a reroll.
Comment #54
chx CreditAttribution: chx commentedhook_language_init is gone from 8.x