Preserve $node->content and $comment->content across multiple node views

makes perfect sense. you can delete that $node->content = array() line and let $node->content = field_attach_view(). Slightly less verbose. after that, is rtbc.

Actually, comment_build_content() and user_build_content() suffer from the same bug. Could you patch those too? Terms do not have this problem.

Ewww. Failures in poll tests. Have fun with that :)

Would it work to add 'rebuild' parameter to node_build_content? This could be TRUE by default, then modify poll module call to not rebuild it.

Let's test doing it this way.

We will need a small test for that.

Looks like right approach. Would love similar fix for user and comment

Doesn't look like user needs it, but this patch covers comment as well.

Indeed you are right. Users already zero our the content property when building.

chasing HEAD...

Committed to HEAD. Thanks!

Sounds like this needed to be ported to D6 as well?

D'oh. :) Yes.

Looks good. This test will confirm that the preceding patch has does it's job.

Awesome, thanks! Just some minor things.

+++ modules/comment/comment.test	3 Nov 2009 07:22:41 -0000
@@ -914,3 +914,35 @@ class CommentRdfaTestCase extends Commen
+  public function testCommentRebuild() {

This needs PHPDoc. Also, this doesn't need to be a public function. Just plain ol' function will do.

+++ modules/comment/comment.test	3 Nov 2009 07:22:41 -0000
@@ -914,3 +914,35 @@ class CommentRdfaTestCase extends Commen
+    $this->drupalLogin($this->admin_user);
+    $this->setCommentSubject(TRUE);
+    $this->setCommentPreview(DRUPAL_OPTIONAL);
+    $this->drupalLogout();
+    $this->drupalLogin($this->web_user);
+    $subject_text = $this->randomName();
+    $comment_text = $this->randomName();
+    $comment = $this->postComment($this->node, $comment_text, $subject_text, TRUE);
+    $comment_loaded = comment_load($comment->id);
+    $this->assertTrue($this->commentExists($comment), t('Comment found.'));
+    
+++ modules/node/node.test	3 Nov 2009 07:22:42 -0000
@@ -1070,3 +1070,26 @@ class NodeFeedTestCase extends DrupalWeb
+    $this->assertFalse(isset($content['test_content_property']), t('Node content was rebuilt'));

This is a lot of dense code. It'd be nice to break it up with an inline comment or two for easier scanning.

+++ modules/comment/comment.test	3 Nov 2009 07:22:41 -0000
@@ -914,3 +914,35 @@ class CommentRdfaTestCase extends Commen
+    // add the property to the content array and then see if it still exists on build
...
+    // this means that the content was rebuilt as the added test property no longer exists
+++ modules/node/node.test	3 Nov 2009 07:22:42 -0000
@@ -1070,3 +1070,26 @@ class NodeFeedTestCase extends DrupalWeb
+  * Create one node and add a piece to its content array. ensure that that piece doesn't remain
...
+    // set a property in the content array so we can test for its exsistance later on
...
+    // if the property doesn't exist it means the node->content was rebuilt

Comments should "Start with a capital, end with a period." Also, wrap at 80 chars.

+++ modules/node/node.test	3 Nov 2009 07:22:42 -0000
@@ -1070,3 +1070,26 @@ class NodeFeedTestCase extends DrupalWeb
+class NodeBuildContent extends DrupalWebTestCase {

PHPDoc.

+++ modules/node/node.test	3 Nov 2009 07:22:42 -0000
@@ -1070,3 +1070,26 @@ class NodeFeedTestCase extends DrupalWeb
+    $this->assertFalse(isset($content['test_content_property']), t('Node content was rebuilt'));

Same comment as the earlier one on the detail of assertion message.

I'm on crack. Are you, too?

Looks good.

Committed to CVS HEAD. Thanks.

I think that this issue are breaking the testbot..

http://qa.drupal.org/pifr/test/32

Very, very odd that those tests passed in the queue but fail now, we should rollback then investigate.

+++ modules/comment/comment.test
@@ -914,3 +914,42 @@ class CommentRdfaTestCase extends CommentHelperCase {
+    $built_content = comment_build($comment_loaded, $this->node);

An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: /batch?id=104&op=do StatusText: OK ResponseText: Fatal error: Call to undefined function comment_build() in /home/dave/Projects/www/drupal7dev/modules/comment/comment.test on line 1226 Call Stack: 0.0001 65992 1. {main}() /home/dave/Projects/www/drupal7dev/index.php:0 0.2395 17111692 2. menu_execute_active_handler() /home/dave/Projects/www/drupal7dev/index.php:22 0.2468 17933444 3. call_user_func_array() /home/dave/Projects/www/drupal7dev/includes/menu.inc:466 0.2468 17933712 4. system_batch_page() /home/dave/Projects/www/drupal7dev/includes/menu.inc:0 0.2468 17933812 5. _batch_page() /home/dave/Projects/www/drupal7dev/modules/system/system.admin.inc:2267 0.2469 17937624 6. _batch_do() /home/dave/Projects/www/drupal7dev/includes/batch.inc:81 0.2469 17937664 7. _batch_process() /home/dave/Projects/www/drupal7dev/includes/batch.inc:163 0.2483 18020240 8. call_user_func_array() /home/dave/Projects/www/drupal7dev/includes/batch.inc:286 0.2483 18020380 9. _simpletest_batch_operation() /home/dave/Projects/www/drupal7dev/includes/batch.inc:0 0.2586 19493852 10. DrupalTestCase->run() /home/dave/Projects/www/drupal7dev/modules/simpletest/simpletest.module:192 6.3158 28085352 11. CommentContentRebuild->testCommentRebuild() /home/dave/Projects/www/drupal7dev/modules/simpletest/drupal_web_test_case.php:442

There is no comment_build function in D7. Might be looking for comment_build_content()?

Powered by Dreditor.

It was renamed to comment_view in #658364: Does build/view/formatter terminology make sense?

comment_view should be comment_view(), otherwise looks great as long as testbot is happy.

Confirmed patch in #35 fixes the test failures. Fixed the documentation and RTBC'ing.

Committed to CVS HEAD. That should fix the tests.

and now, return to D6

Did this ever get fixed in D6? We have a bad apache memory leak I'm trying to track down.

Err: I just come across this from #564632: Cruft in $page due to node_build, comment_build, etc.. A D8 patch is in that queue, but I will close it once the test bot has run.

Use-case: A node view references something that also renders nodes through the node system. One of these nodes is the parental container itself, meaning that the $node->content array is reset and the parents build information is rendered NULL. This leads to a fatal error:

Fatal error: Unsupported operand types in modules/node/node.module on line 1362

So this was my workaround.

  // To support recursive calls, save any existing build information.
  $existing_content = isset($node->content) ? $node->content : NULL;
  
  // Populate $node->content with a render() array.
  node_build_content($node, $view_mode, $langcode);
  $build = $node->content;

  // Reset $node->content with any existing build information.
  $node->content = $existing_content ;
 

A Drupal 7 patch is attached.

For D8: Note unlike that other comments, rather than rebuilding from scratch, I would suggest adding in the context of the view mode to the content array and not resetting. Thus caching a copy and enabling reuse rather than rebuilding each time it is required. Only common use-case would be on listings that allow duplicates, like events with multiple sessions, where the same listing can show multiple times.

[Edit] This is one of the reasons why calendars in Drupal always appear so slow :( [/Edit]

I removed the white space before the semi-colon.

#45: drupal-recursive-error-when-rebuilding-node-build-564642-45.patch queued for re-testing.

I'm a little confused by #44. We need to whatever bug still exists in D8 first, even if we come up with a better fix later. Also, let's get an automated test that demonstrates the functional bug.

I think the easiest way would be to create a simple filter tag that renders a node teaser. Then enable this filter on a node body field. Edit a node and insert this tag with the nid of the node being edited. It is a really obscure bug!

I triggered it by embedding an insert_view tag into the content of a node. This showed a teaser listing of recently updated content. If the node that had this tag was rendered in this listing, the fatal error occurred. [Mind you there were bigger bugs to resolve in the insert_view module to even get this far - recursive rendering of the same view tag, I've posted a patch in that modules issue queue to prevent this.]

(#45 is working nicely for those that just need to fix it)

Does anyone know if d8 has this bug?

Got bitten by this again

I'm going to go out on a limb and suggest that this is not relevant for D8 because of the way it handles multiple builds within the entity controller.

Rerolled #45 against 7.x-dev and queued for testing.

RTBC - though I think we really should write a test for it, so I probably will need to put it to CNW next when I review the RTBC queue with my maintainer head on ;).

I just want to signal that I think that this is ready.

We indeed will need a test for this, setting to Needs work for that.