template_preprocess_node assumes node has fields attached

#2: template_preprocess_node.patch queued for re-testing.

This cleaned up my log messages nicely ;-) And i was getting the error on a content type that did have fields.

Thanks, worked great for me, applied on 7.0 official.

Worked for me, Drupal 7.0

+1

Patch didn't apply. Re-rolled using "git format-patch"

Also needs tests to ensure this error does not reoccur. Unfortunately, learning Simpletest is still on my to-do list. Tagging.

Hmm...
Trying to recreate the problem without writing a custom module:

1. Created a content type "foo".
2. Removed the body field.
3. Turned off all the optional stuff I could find.
4. Created a node of that type with title "bar" ($nid = 1058)
5. Viewed the node
6. Looked at error logs

Nothing.

Inserting a watchdog() into the template_preprocess_node() function reveals that there is still a 'links' element, even though its rendered version doesn't contain visible content.

Is there a way to duplicate this problem without writing a custom module?

Expanded patch context makes it clear why the change is necessary -- $variables['content'] is assumed to exist only six lines further down.

#11: template_preprocess_node-705892-11.patch queued for re-testing.

Patch didn't apply. Re-rolled using "git format-patch"

Maybe you should check your version of patch...

$ curl http://drupal.org/files/issues/template_preprocess_node_2.patch | patch -p0
patching file modules/node/node.module
Hunk #1 succeeded at 1459 with fuzz 2 (offset 85 lines).

The patch in #2 is still RTBC in my book (as it was set to in #4).

git clone git://drupalcode.org/project/drupal.git ; cd drupal; git checkout 7.x ; curl http://drupal.org/files/issues/template_preprocess_node_2.patch | git apply
remote: Counting objects: 118209, done.
remote: Compressing objects: 100% (27790/27790), done.
remote: Total 118209 (delta 87494), reused 114738 (delta 84049)
Receiving objects: 100% (118209/118209), 35.96 MiB | 241 KiB/s, done.
Resolving deltas: 100% (87494/87494), done.
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   712  100   712    0     0    970      0 --:--:-- --:--:-- --:--:--  5836
error: node/node.module: No such file or directory

Trying again, using "git apply -p0" instead of "git apply"

curl http://drupal.org/files/issues/template_preprocess_node_2.patch | git apply -p0
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   712  100   712    0     0   1093      0 --:--:-- --:--:-- --:--:--  6414
error: patch failed: modules/node/node.module:1374
error: modules/node/node.module: patch does not apply

Trying the patch in #11 instead:

curl http://drupal.org/files/issues/template_preprocess_node-705892-11.patch | git apply
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1053  100  1053    0     0   1099      0 --:--:-- --:--:-- --:--:--  9000

Maybe you should check your version of patch.

The patch in #11 was prepared according to the Git instructions linked from the project page. Maybe you should read them.

Bikeshedding aside, I agree on the RTBC, but it still needs tests.

Is there any way to recreate this problem using simpletest?

It's a stupid notice, so I don't see the need for a test. That said, this should not have that many spaces:

   // Helpful $content variable for templates.
+  $variables['content']   = array();

Let's get a reroll, please.

Rerolled per Damien in #17. Patch credit goes to jherencia in the original post.

I agree that no test is required here.

The patch in #11 was prepared according to the Git instructions linked from the project page. Maybe you should read them.

Sorry to upset you. My apologies. I guess git is stricter than patch in it's context requirements—adding -C1 to the git apply command successfully applies the patch.

I got the same problem. It works great for me too, applied on 7.0 official. Thanks !
Patch applied : 705892-Fixed-template_preprocess_node-assumes-.patch

Yep, I'm using the 'needs backport' tag to keep track of issues that need to go into D7 as well as D8 (as opposed to new Drupal 8 issues that will never be backported).

G'day guys,

I really appreciate the work that has gone into finding a solution for this and developing a patch. Thanks.

Now what do I do with the patch? I am not a coder and have only the barest knowledge of php. I am a website designer with a good working knowledge of html. I am not afraid of making changes if I know what to do.

Please pardon my ignorance, but I'd appreciate it if somebody could tell me which bits of the patch go where in what file. That would be really helpful.

I am running D7 with Drupal Commerce using mainly the latest dev versions of the modules.

Thanks

Willem

bfroehle's patch fixes the issue in Drupal 7. Any chance that it'll be rolled into the next release?

@dale386 -- yes, but not before next week. See http://drupal.org/node/1050616#comment-4471696

Just a slight observation, is it possible to set content before template_preprocess_node(). While this does not happen in the core Drupal code, if a modules weight is less than the node system weight, then this module could potentially add something to the content array, and this change will happily nuke that modules additions.

So maybe this is better?

+  $variables += array('content' => array());

I'd have to concur with Alan D that the addition should be non-destructive. Someone might have reason to fill up the content array before template_preprocess_node gets to it, or their module just happens to get called first.

BUT, I don't actually know if that even partially probable or possible.

If it is possible though, then that fact should be taken into account. Or specifically documented that adding anything to the content key prior to X will mean it is deleted.

This change didn't manage to make it into Drupal 7.2 - is there any way to make sure that it does make it into the next version? I've attached a git formatted patch of both versions just for fun.

The += version
705892-template_preprocess_node_29_d7.patch

Overwrite version
705892-template_preprocess_node_29b_d7.patch

Ok then, here's the D8 version with +=, credited to bfroehle (I just manually edited his patch to maintain the header)

The D7 and D8 patches are now identical except for offset. The 705892-template_preprocess_node_29_d7.patch from my post above is the += one.

Should I be setting this to needs review, or leaving it as reviewed? It is a single line after all, and should not cause issues to >PHP5.

edit: I just discovered that RTBC is not "Ready To Be Committed", but "Reviewed & Tested by the Community" so this should probably be set to "Needs Review". oops.

Blah .. it's been committed to D8 already! Shouldn't the status have been changed when that happened? Or is the D7 work causing the status not be able to be changed?

Here's a new patch to change D8 so that it's non-destructive too.

.. the exact same patch with status set to needs review. So frustrating. Sorry for the spam.

This was the commit - http://drupalcode.org/project/drupal.git/commitdiff/7d1738506cfa81eb3e70...

Was a duplicate issue with the same patch as earlier ones on this issue.

This new one looks fine, in fact it's possible the other patch would have broken performance_hacks, although I don't have time to check now, marking RTBC.

I haven't followed it completely through, but it doesn't look like they tread on each other. Strangely, the $node->content gets built twice, in two different modes; one as a render array, the other as food for the node template file. (This is in both the normal node_view and the performance_hacks one). This patch is in the latter.

Doing normal things while rendering nodes would have been fine without this little change, but doing strange things like calling node_view with a specially crafted node->content would have been wiped out.

How do we get this into D7?

Oops. Mea culpa. :( I remember writing a reply to this issue, but Firefox must've crashed. That reply said basically that I'd considered the fact that something might be trying to set the content variable before it gets to this point but that by investigating the stacktrace I couldn't actually find such a point.

Nevertheless, this seems like a fine follow-up to be extra safe.

Committed to 8.x and 7.x. Thanks!

Removing tags (needs backport to D7 queue cleanup)