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.Over in #571654: Revert node titles as fields, Morbus said:
I think this patch might have reverted a previously working behavior too: the ability for a hook_node_view() to change $node->title at a page level. If you take a look at the diff at http://drupalcode.org/viewvc/drupal/drupal/modules/node/node.module?r1=1..., you'll see that node_page_view() has been changed. Prior to the reversion, the workflow was a) call node_show(), b) set the title. In the reverted/commited code, that's reversed: we a) set the title, then b) call node_show(). Since node_show() calls all the 'view' hooks, the reverted patch now makes it impossible to set the node title at the page level (both in the HTML [title] and in the page.tpl.php $title).
Per chx: "Morbus|kids: if this is so that would be a tad bit surprising and of course mandating a critical bugreport" (though, instead of making a bug report, I added it here; splice it out if you think it should).
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | node_title.patch | 1.03 KB | catch |
| #9 | 761620-6.patch | 1.23 KB | jpmckinney |
| #6 | 761620-6.patch | 1.24 KB | jpmckinney |
| #4 | 0001-Follow-up-to-revert-node-titles-as-fields-allow-hook.patch | 1.54 KB | marvil07 |
| #1 | revert-node-page-view.patch | 1.11 KB | JohnAlbin |
Comments
Comment #1
JohnAlbinHere's a patch that reverts node_page_view(). I'm not sure if that actually fixes the issue Morbus points out, though.
Comment #3
sunsubscribing
I thought we removed [$langcode][0]['value'] from $node->title in the other issue?
Powered by Dreditor.
Comment #4
marvil07 CreditAttribution: marvil07 commentedjust let it apply :-)
Comment #6
jpmckinney CreditAttribution: jpmckinney commentedWell, this gets tests passing. Not sure if this is the best fix to the patch. But, something to think about.
Comment #7
jpmckinney CreditAttribution: jpmckinney commentedComment #9
jpmckinney CreditAttribution: jpmckinney commentedComment #10
marvil07 CreditAttribution: marvil07 commentedit seems fine
Comment #11
webchickcatch says this isn't the right way to go, since node title isn't a field anymore. He's going to re-roll it shortly.
Comment #12
catchPrevious patch still accessed $node->title in the field structure, which we don't have.
Comment #13
moshe weitzman CreditAttribution: moshe weitzman commentedPersonally, I don't we should be calling drupal_set_title() at all here. Let the hook_node_view call it if it needs to. The title should be set by a title_callback in the menu system. Thats the point of #737792: Use a "title callback" for node/%node instead of drupal_set_title(). AFAICT, this is won't fix.
Comment #14
catchThe latest patch at #737792: Use a "title callback" for node/%node instead of drupal_set_title() doesn't remove drupal_set_title() from node_page_view(), so I think this issue stands until that one is sorted properly. I have a bad feeling that it won't be possible to do a proper fix without being able to set the page title via the render API (something along the lines of '#attached']['drupal_set_title']).
Comment #15
plachsubscribe
Comment #16
catchI'm moving this back to RTBC, we can try to remove it altogether in #737792 but I'd rather see the regression out of the way first.
Comment #17
webchickHonestly, I like the fix at #737792: Use a "title callback" for node/%node instead of drupal_set_title() better. It seems to be both addressing this issue and fixing an oversight. So marking this won't fix.
Comment #18
sun