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.Problem/Motivation
After commit 301dc87 which was included in 2.4 and 2.5, with some modules installed it is possible that tokens to be built in a way that leaves the site without titles or with the wrong titles. This is caused by 2 things:
1) relying on hook_init() to include modules made it possible to call hooks
before our extra module includes have been included.
2) static caching of page title meant we couldn't rebuild it after the system
state had changed. For example, being called by OpenSearch and then updating
in preprocess_page with the value set by drupal_set_title() during page
building.
Proposed resolution
By moving the logic for including our extra module files into an explicitly called function and removing static caching from page_title_get_title() we work around these two problems and restore titles in a minimally invasive fasion.
User interface changes
none
API changes
The calling cost of page_title_get_title() has slightly increased for additional calls. I think this is mitigated by token's(somewhat problematic) caching and the fact that it shouldn't be called often as is noted in the functions original documentation. Also, the code should be cheap enough to not pose a problem in the general case.
Original report by [neclimdul]
After upgrading from 2.4 to 2.5 or 2.-dev I no longer get any page titles. After review, it seems to be token_replace_multiple causing the problem. I get a page_title_pattern(generally [page-title]) but token_replace_multiple() always returns an empty string on replacement. I haven't researched past this.
Note: I was actually upgrading from 2.3, not 2.4.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | page_title.zip | 8.01 KB | fugazi |
| #12 | page_title-2.4_broken_titles-1205356-2.patch | 4.49 KB | neclimdul |
| #10 | page_title-2.4_broken_titles-1205356-1.patch | 495.36 KB | neclimdul |
Comments
Comment #1
nicholasThompsonwhich version of Tokens are you using?
Comment #2
neclimdul6.x-1.16
Comment #3
neclimdulAlso, using Pressflow 6.22.104. That shouldn't make a difference but its a possibly relevant fact.
Comment #4
nicholasThompsonThis is unusual; the module comes with a SimpleTest script which passes on my Drupal 6.22 install (which is obviously patched to run SimpleTest)... The test includes several variations on token templates.
Do you have any page caching enabled? If so, is it "normal" or "aggressive"? Page Title uses
hook_initto include its module-level files which implementhook_page_title_alterandhook_page_title_pattern_alteron behalf of modules such as Node, Taxonomy, Blog and Forum. These hooks are the ones which handle getting/setting the appropriate values.I assume
[page-title]is the default/fall-back pattern?Are the tokens being replaced at all? Eg, if you had
[page-title] | [site-name], does it replace[site-name](this is a Global Token which should ALWAYS have a value)...Comment #5
fugazi CreditAttribution: fugazi commentedsame problem after upgade on Title Page 6.x-2.5-page title is no longer available. [site name] is available. I use Token 6.x-1.16.
[site-name] | [site-slogan] works well.
I can back to Title Page 6.x-2.3 without problems? because everything worked.
Comment #6
fugazi CreditAttribution: fugazi commentedAre already known more information. It is very important to me. Can I make one without problems back to 6.x-2.3 implementation? many thanks
Comment #7
neclimdulFound the problem. For me its caused by opensearch but it could actually be any module calling token functions early. Because opensearch's _init hook is firing before page_title, the extra module includes haven't been loaded and the drupal_alter('page_title') in the token values callback fails to find the needed alters to populate the default title.
So, we could solve this with module weights but that always feels like the wrong solution so I'm thinking maybe we get rid of page_title_init and move the code there closer to the code using it such as calling a _page_title_module_init() function or some such. I've got a patch mostly implementing this but I'm still running into a bug on pages with views titles(possible just views with titles from arguments).
Comment #8
neclimdulSo found the second part of the problem. Since tokens are being built /very/ early, other modules(like views) are unable to call drupal_set_title and have that affect page_title through the alter in page_title.page_title.inc.
There's 2 solutions I see here.
1) Move our token to a different type so we have more granular control of when its built.
2) Remove the static from page_title_get_title and let the title be rebuilt each time.
Being as page_title_get_title() shouldn't be called very often, I don't really see a problem removing the static and this would allow page-title to continue to be used in other token replacements so it might be the best option. Thoughts?
Comment #9
neclimdulAdded issue summary
Comment #10
neclimdulPatch with outlined solution. There is a bit of an out of scope fix included that moves the admin report view css into a views hook instead of hook_init. I will happily remove that and provide it in a separate issue.
Comment #11
fugazi CreditAttribution: fugazi commentedI'm back to 2.3 and it works perfectly. I'm sorry I can not use the new version. But with me it does not work. These contributions will follow here to maybe be able to do another update. Many thanks for the great module. Sorry for my bad english
Comment #12
neclimdulfugazi np. Could you test my patch and see if it fixes your problem too?
Also, updated patch that's not all messed up. Don't know what happened with the other patch.
Comment #13
fugazi CreditAttribution: fugazi commentedI've edited page_title.module 2.5. but get the following error message.
Parse error: syntax error, unexpected $end in sites/all/modules/page_title/page_title.module on line 891I've changed page_title.module attached
Comment #14
nattsJust to let you know the patch in #12 fixed the latest (6.x-2.5) release for me.
Comment #15
neclimdulI'm loath to mark this RTBC myself but #14 fixes the problem for natts and I. I think fugazi just had trouble applying the patch so this should be ready to go.
@commiters the problem was that the static in page_title_get_title could be set by modules too early in the page request and then was not able to be reset. The patch removes this static and relies on the statics in the underlying functions to keep the limited number of calls to it performant. I think this should be acceptable and allows a behavior that at least for me was more consistent with the core title features.
Comment #16
d.sibaud CreditAttribution: d.sibaud commentedpatch in #12 fixed the latest (6.x-2.5) release also for me.
Comment #17
jeni_dc CreditAttribution: jeni_dc commentedPatch in #12 worked for me, too. 6.x-2.5
Comment #17.0
jeni_dc CreditAttribution: jeni_dc commentedwrite initial summary with solution and outlining of core issues involved.
Comment #18
DamienMcKennaThe patch doesn't apply cleanly to the 6.x-2.x branch so I couldn't commit it. The D6 branch is no longer supported, so while I appreciate the effort, no further work will be done on this module.