node_body_field does not check $type->has_body [#223771]

The settings page for all content types (including blog entries) states clearly that to remove the body field of a node, the label must be blanked. However, while content types handled by the node module will display this expected behavior, removing the body label on the blog entry will only remove the label, not the field.

While node_content_form() checks the has_body property before generating a field,

  if ($type->has_body) {
    $form['body_field'] = node_body_field($node, $type->body_label, $type->min_word_count);
  }

blog_form() does not do so:

  $form['body_field'] = node_body_field($node, $type->body_label, $type->min_word_count);

node_body_field() does not actually contain any logic regarding field labels:

function node_body_field(&$node, $label, $word_count) {

  // Check if we need to restore the teaser at the beginning of the body.
  $include = !isset($node->teaser) || ($node->teaser == substr($node->body, 0, strlen($node->teaser)));

  $form = array(
    '#after_build' => array('node_teaser_js', 'node_teaser_include_verify'));

  $form['#prefix'] = '<div class="body-field-wrapper">';
  $form['#suffix'] = '</div>';

  $form['teaser_js'] = array(
    '#type' => 'textarea',
    '#rows' => 10,
    '#teaser' => 'edit-body',
    '#teaser_checkbox' => 'edit-teaser-include',
    '#disabled' => TRUE,
  );

  $form['teaser_include'] = array(
    '#type' => 'checkbox',
    '#title' => t('Show summary in full view'),
    '#default_value' => $include,
    '#prefix' => '<div class="teaser-checkbox">',
    '#suffix' => '</div>',
  );

  $form['body'] = array(
    '#type' => 'textarea',
    '#title' => check_plain($label),
    '#default_value' => $include ? $node->body : ($node->teaser . $node->body),
    '#rows' => 20,
    '#required' => ($word_count > 0),
  );

  $form['format'] = filter_form($node->format);

  return $form;
}

I submit this issue to blog.module as it could easily solve the problem by emulating node.module in checking for has_body before generating a field. However, perhaps this check should be centralized to node_body_form, which would work to limit these errors in nodes by contrib modules. node_body_form already gets a node, which has a type - checking this type for the presence of a body field would make much sense.

Comment	File	Size	Author
#17	node-body-field-hasbody-223771-17.patch	1.46 KB	cburschka
#16	node-body-field-hasbody-223771-16.patch	1.43 KB	cburschka
#14	node-body-field-hasbody-223771-14.patch	1.43 KB	cburschka
#10	node-body-field-hasbody-223771-10.patch	1.44 KB	amitaibu
#2	node-body-field-hasbody-223771-2.patch	1.82 KB	cburschka

Comments

Comment #1

cburschka

they

commented 19 February 2008 at 13:03

Note: My last references to node_body_form should be node_body_field, naturally.

Comment #2

cburschka

they

commented 19 February 2008 at 13:18

Title:	Blog entries have body fields even if body was disabled	» node_body_field does not check $type->has_body
Component:	blog.module	» node.module
Status:	Active	» Needs review

Status	File	Size
new	node-body-field-hasbody-223771-2.patch	1.82 KB

On second thought, perhaps the node_body_field solution makes even more sense than the blog_form fix. I'm moving it into the node.module queue now, and submitting a patch to node.module.

The main advantage of this fix: It ensures expected behavior on node forms without requiring all node modules to reimplement central logic. If the content type settings state that I can disable the body field, then the body field should be gone regardless of what module is handling this node.

(Attached patch also removes the now redundant check from node_content_form, as well as a typo in a comment (mutlistep) I happened to notice.)

Comment #3

Steve Dondley commented 7 October 2008 at 00:34

+1 This is needed. Too bad it looks like it got overlooked.

Comment #4

Steve Dondley commented 7 October 2008 at 00:47

Version:

6.x-dev

» 6.4

Comment #5

Anonymous (not verified) commented 7 October 2008 at 14:50

Steve, have you applied the patch and it works properly? If so, change the status to "patch (reviewed & tested by the community).

Comment #6

Steve Dondley commented 9 October 2008 at 03:09

I have not tried the patch. I used the suggestion in the original post and modified blog.module. So I'm uncomfortable saying this has been tested and reviewed. I think someone more familiar with the base code should weigh in on this one. Though I can see no reason why the code above would cause problems, I've been burned enough by the law of unintended consequence to say anything definitively.

Comment #7

livingegg commented 9 October 2008 at 17:30

This bug also applies to version 5.x. It would be nice to have a fix.

Comment #8

damien tournoud commented 9 October 2008 at 18:15

Version:	6.4	» 7.x-dev
Status:	Needs review	» Needs work

Bugs get fixed in the current development version, then backported.

The patch in #2 is an API change, because (1) node_body_field() behavior is modified, and (2) because $form now contains the 'body_field' key unconditionally. As such, the change should not go in D6 as is. Just changing blog_form should be enough for D6.

Comment #9

mrgoltra commented 2 January 2009 at 09:21

subscribing. I would really love to upgrade my site to 6 but I can't until this issue is fixed.

Comment #10

amitaibu

Hebrew

Israel

commented 11 January 2009 at 10:53

Status:

Needs work

» Needs review

Status	File	Size
new	node-body-field-hasbody-223771-10.patch	1.44 KB

Re-roll Arancaytar's patch against HEAD with slight modification (return array()).
I've applied patch, and it works.

Comment #11

11 January 2009 at 11:30

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #12

hanoii

Spanish

🇦🇷UTC-3

commented 13 January 2009 at 16:29

I have found this as well on one of my projects and I wanted to submit a patch for D5 which involves changing just blog_form() as suggested in #8. What's the proper way of submit such a patch? Shall I create a new issue or submit it here?

Comment #13

nonsie

she/her

Estonian

commented 30 March 2009 at 23:15

Subscribing

Comment #14

cburschka

they

commented 9 April 2009 at 18:03

Status:

Needs work

» Needs review

Status	File	Size
new	node-body-field-hasbody-223771-14.patch	1.43 KB

I cannot apply the patch in #10, and it seems neither can pifrbot.

Here's a reroll.

Comment #15

mikejoconnor commented 23 May 2009 at 03:38

Status:

Needs review

» Needs work

Arancaytar,

I think this is a great approach. Just two small critiques. The comments run over 80 characters, and I don't think we put comments on the same line as other code. I looked through a few core files and all of the comments are above subject of the comment. I'm marking as code needs work, only because of formatting. I would suggest something like this:

function node_body_field(&$node, $label, $word_count) {

  $type = node_get_types('type', $node);

  // Do not generate a body field if the type does not specify one.
  if (!$type->has_body) {
    return array();
  }

Comment #16

cburschka

they

commented 23 May 2009 at 14:14

Status:

Needs work

» Needs review

Status	File	Size
new	node-body-field-hasbody-223771-16.patch	1.43 KB

Whoops. You're right, of course. I've remarked the same point on countless patches! :P

Here we go again.

Comment #17

cburschka

they

commented 23 May 2009 at 14:20

Status	File	Size
new	node-body-field-hasbody-223771-17.patch	1.46 KB

In fact, since we only use $type for that one check, I think PHP 5 might allow node_get_types(...)->has_body, but I'm not sure. I also don't know if it's proper code.

Here is a separate patch that saves a few lines by doing that. I got no syntax errors, but use the one above if the bot complains.

Comment #18

dries commented 25 May 2009 at 09:35

This patch makes sense to me, and all the tests pass. I'm marking this RTBC and will proceed committing it later today.

Comment #19

cburschka

they

commented 25 May 2009 at 13:04

Status:

Needs review

» Reviewed & tested by the community

Yay! :)

Comment #20

dries commented 25 May 2009 at 15:39

Status:

Reviewed & tested by the community

» Fixed

Committed to CVS HEAD. Thanks!

Comment #21

8 June 2009 at 15:40

Status:

Fixed

» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Comment #22

grendzy commented 11 May 2010 at 16:04

Version:	7.x-dev	» 6.x-dev
Status:	Closed (fixed)	» Patch (to be ported)
Issue tags:		+Needs backport to D6

/**
 * Implementation of hook_form_alter().
 */
function MYMODULE_form_alter(&$form, $form_state, $form_id) {
  switch ($form_id) {
    case 'blog_node_form':
      unset ($form['body_field']) ;
      break;
  }
}

Comment #26

Anonymous (not verified) commented 9 October 2012 at 14:22

Status:

Patch (to be ported)

» Closed (won't fix)

I'm going to guess this is a won't fix for D6, since D6 commits focus on security fixes at this point.

Comment #27

Anonymous (not verified) commented 9 October 2012 at 16:47

Version:	6.x-dev	» 7.x-dev
Status:	Closed (won't fix)	» Closed (fixed)

Changing issue status to reflect that it was fixed in 7.x.

node_body_field does not check $type->has_body

Comments

Comment #1

Comment #2

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

Comment #8

Comment #9

Comment #10

Comment #11

Comment #12

Comment #13

Comment #14

Comment #15

Comment #16

Comment #17

Comment #18

Comment #19

Comment #20

Comment #21

Comment #22

Comment #23

Comment #24

Comment #25

Comment #26

Comment #27

News items

Our community

Documentation

Drupal code base

Governance of community