Body Class module helps the content editors / admins to update custom classes to body tags in each nodes. Multiple classes can be added by adding extra space. Also one configuration page helps to view all the custom body classes used in the entire site.

Manual reviews of other projects

Project link

https://www.drupal.org/project/body_class

Git instructions

git clone --branch 7.x-1.x https://git.drupalcode.org/project/body_class.git

Comments

JaseerKinangattil created an issue. See original summary.

jaseerkinangattil’s picture

Issue summary: View changes
jaseerkinangattil’s picture

Status: Active » Needs review
avpaderno’s picture

Issue summary: View changes
jaseerkinangattil’s picture

Issue summary: View changes
Issue tags: +PAreview: review bonus
jaseerkinangattil’s picture

Issue summary: View changes
avpaderno’s picture

Issue summary: View changes
jaseerkinangattil’s picture

Team, Please review

ameer khan’s picture

Status: Needs review » Reviewed & tested by the community

I did a manual review & it works as described.

avpaderno’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +PAreview: single application approval, +PAreview: security

It would be helpful to describe the differences between this module and the Assets Injector module or the CSS Injector module, or the reasons why the users should choose this module instead of one of the other two modules.

function body_class_permission() {
  return [
    'Administer Body Class' => [
      'title' => t('Administer Body Class'),
      'description' => t('Configure permission for Body Class'),
    ],
  ];
}

That isn't the correct description of the permission, which doesn't allow to configure permissions, but to set the CSS classes associated to nodes.

function body_class_help($path, $arg) {
  switch ($path) {
    case 'admin/config/development/body_class':
      $output = '';
      $output .= '<h1>' . t('The  <a href="@drupal" target="_blank">body_class</a> Module', ['@drupal' => 'http://drupal.org/project/body_class']) . '</h1>';
      $output .= '<p>' . t('Body Class is a simple module that allows users to add custom CSS classes
to any node through the node/add interface. The respective classes will be appearing in body tags.') . '</p>';
      $output .= '<p>' . t('Uninstallation of the module will delete all the custom classes you added and cannot be recovered.') . '</p>';
      return $output;
  }

}

body_class isn't the module name, which is Body Class.

function body_class_menu() {
  $items['admin/config/development/body_class'] = [
    'title' => 'Body Class',
    'description' => 'Configure how the modules page looks and acts.',
    'access arguments' => ['Administer Body Class'],
    'page callback' => 'body_class_submissions',
  ];
  return $items;
}

That isn't the correct description of the purpose of that page.

  foreach ($result as $res) {
    $nid = $res->nid;
    $node = node_load($nid);
    $node_title = l($node->title, 'node/' . $nid . '/edit');
    $rows[] = [
      $res->nid,
      $node_title,
      $res->css_class,
    ];
  }

The code should first check the user who is accessing the configuration page has the permission to edit the node.

  if (!empty($rows)) {
    $output = theme('table', [
      'header' => $header,
      'rows' => $rows,
      'attributes' => ['id' => 'sort-table'],
    ]);
    $output .= theme('pager');
  }
  else {
    $output .= t('No results found.');
    drupal_set_message(t('No results found.'));
  }

The else branch is not necessary, as the empty property can be used for the message to show when the table is empty. as in the following code used from a Drupal core module.

  $output .= theme('table', array(
    'header' => $header,
    'rows' => $rows,
    'empty' => t('No feeds available. <a href="@link">Add feed</a>.', array(
      '@link' => url('admin/config/services/aggregator/add/feed'),
    )),
  ));

Instead of rendering the table, the code could also return a render array, as the following code does.

  $build['dblog_table'] = array(
    '#theme' => 'table',
    '#header' => $header,
    '#rows' => $rows,
    '#attributes' => array(
      'id' => 'admin-dblog',
    ),
    '#empty' => t('No log messages available.'),
  );
  $build['dblog_pager'] = array(
    '#theme' => 'pager',
  );
  return $build;

The latter code is preferable, as it allows a third-party module, or Drupal core to cache the render array setting the #cache property.

/**
 * Accessor for css_class information.
 */
function body_class($node) {
  $attributes = body_class_attributes($node);
  return $attributes;
}

Since the module uses directly the body_class_attributes() function, and body_class() is an one-line function (whose body can be merely written as return body_class_attributes($node);), that function can be removed.

  if (isset($form['#node']) && ($form_id == $form['#node']->type . '_node_form') &&
    user_access('administer nodes')) {
    // Add form elements in vertical tab to node settings.
    $form['body_class'] = [
      '#type' => 'fieldset',
      '#title' => t('Body Class settings'),
      '#description' => t('Assign CSS classes to the node in the body tag.'),
      '#collapsible' => TRUE,
      '#collapsed' => TRUE,
      '#access' => user_access('administer nodes'),
      '#weight' => 80,
      '#tree' => TRUE,
      '#group' => 'additional_settings',
      '#tree' => TRUE,
      '#attributes' => ['class' => ['node-class-form']],
    ];

    // Omissis
  }

Either user_access('administer nodes') is removed from the if statement, or the #access line is removed. (I would rather opt for the first solution.)

function body_class_node_insert($node) {
  if (isset($node->body_class['css_class'])) {
    $nid = $node->nid;
    $class = check_plain($node->body_class['css_class']);
    body_class_upsert($nid, $class);
  }
}

User-entered values are eventually sanitized when rendered in the output, not when saved. The module is doing exactly the opposite. (See also the following lines taken from body_class_preprocess_html().)

  if (!empty($nid)) {
    $node = node_load($nid);
    $class = body_class_attributes($node);
  }
  $variables['classes_array'][] = $class;
    if (!db_query("SELECT COUNT(*) FROM {body_class} WHERE nid = :nid", [':nid' => $nid])->fetchField()) {
      db_insert('body_class')->fields([
        'nid' => $nid,
        'css_class' => $class,
      ])->execute();
    }
    else {
      db_update('body_class')
        ->fields(['css_class' => $class])
        ->condition('nid', $nid)
        ->execute();
    }

Instead of using those lines, the code should use db_merge(), whose purpose is described in Merge queries using db_merge.

name = Body Class
description = Adds custom class to body tag in each node
core = 7.x
package = Other
configure = admin/config/development/body_class

While sites are surely running on a PHP version higher than 5.6, the module should make explicit its dependency from a more recent PHP version. cPanel used from some service providers still allows to select PHP 4.4 for running a site.

avpaderno’s picture

avpaderno’s picture

Priority: Normal » Minor
avpaderno’s picture

Status: Needs work » Closed (won't fix)
Issue tags: -PAreview: review bonus

I am closing this application since there have been replies in the past 5 months.

avpaderno’s picture

Status: Closed (won't fix) » Closed (duplicate)
Issue tags: -PAreview: single application approval
Related issues: +#3485130: [1.0.x] Social Auth Entra ID
avpaderno’s picture