For context, I'm integrating hierarchical selects with homebox module in a block settings form. The form uses hierarchical select as a FAPI element, and it works pretty well. However, once two blocks/forms are put on the same page I start having issues with the widget in the first form trying to operate the widget in the second form. This is because the two form elements share the same id, which is generated using _hs_process_determine_hsid. So:

The way _hs_process_determine_hsid currently assigns hsids doesn't work correctly when you have 2 copies of the same form, each containing a hierarchical select widget.

This is because $form_state['storage'] is not shared between the different forms.

This can be fixed by storing the hsid in $_SESSION instead of in $form_state, which looks like it was the say it was done in 6.x from the looks of <>.

Am I missing something here?

diff --git a/sites/all/modules/contrib/hierarchical_select/hierarchical_select.module b/sites/all/modules/contrib/hierarchic
index 731267d..4bd4fba 100644
--- a/sites/all/modules/contrib/hierarchical_select/hierarchical_select.module
+++ b/sites/all/modules/contrib/hierarchical_select/hierarchical_select.module
@@ -349,11 +349,11 @@ function _hs_process_determine_hsid($element, &$form_state) {
   // generate a new one based on the last HSID used (which is
   // stored in form state storage).
   if (!isset($element['#value']) || !is_array($element['#value']) || !array_key_exists('hsid', $element['#value'])) {
-    if (!isset($form_state['storage']['hs']['last_hsid'])) {
-      $form_state['storage']['hs']['last_hsid'] = -1;
+    if (!isset($_SESSION['storage']['hs']['last_hsid'])) {
+      $_SESSION['storage']['hs']['last_hsid'] = -1;
     }
-    $form_state['storage']['hs']['last_hsid']++;
-    $hsid = $form_state['storage']['hs']['last_hsid'];
+    $_SESSION['storage']['hs']['last_hsid']++;
+    $hsid = $_SESSION['storage']['hs']['last_hsid'];
   }
   else {
     $hsid = check_plain($element['#value']['hsid']);
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

samhassell’s picture

Ok here's a patch which makes a couple of changes allowing 2 things:

1) Get hs elements working correctly when multiple are loaded on one page via AJAX.

2) Port hs_taxonomy_term_count_nodes to Drupal 7.

I'm sure some improvements can be made to the patch, I'm not full bottle on how hs is working.

Sorry for combining what should be 2 patches :P

samhassell’s picture

Status: Active » Needs review

Little description of the changes:

+++ b/hierarchical_select.jsundefined
@@ -471,7 +471,14 @@ Drupal.HierarchicalSelect.update = function(hsid, updateType, settings) {
-      var lastUnchanged = parseInt(settings.select_id.replace(/^.*-hierarchical-select-selects-(\d+)$/, "$1")) + 1;
+
+      if (settings.select_id.substring(settings.select_id.length - 3, settings.select_id.length - 1) == '--') {
+        var lastUnchanged = parseInt(settings.select_id.replace(/^.*-hierarchical-select-selects-(\d+)--.$/, "$1")) + 1;
+      }
+      else {
+        var lastUnchanged = parseInt(settings.select_id.replace(/^.*-hierarchical-select-selects-(\d+)$/, "$1")) + 1;
+      }

When you have multiple hs elements, Drupal adds --1, --2, etc to the end of the #id. Updated to handle this, though it would be better to stop Drupal doing this in the first place?

+++ b/hierarchical_select.moduleundefined
@@ -349,11 +349,11 @@ function _hs_process_determine_hsid($element, &$form_state) {
-    if (!isset($form_state['storage']['hs']['last_hsid'])) {
-      $form_state['storage']['hs']['last_hsid'] = -1;
+    if (!isset($_SESSION['storage']['hs']['last_hsid'])) {
+      $_SESSION['storage']['hs']['last_hsid'] = -1;
     }
-    $form_state['storage']['hs']['last_hsid']++;
-    $hsid = $form_state['storage']['hs']['last_hsid'];
+    $_SESSION['storage']['hs']['last_hsid']++;

This is the change outlined in the main issue.

+++ b/hierarchical_select.moduleundefined
@@ -349,11 +349,11 @@ function _hs_process_determine_hsid($element, &$form_state) {
-    if (!isset($form_state['storage']['hs']['last_hsid'])) {
-      $form_state['storage']['hs']['last_hsid'] = -1;
+    if (!isset($_SESSION['storage']['hs']['last_hsid'])) {
+      $_SESSION['storage']['hs']['last_hsid'] = -1;
     }
-    $form_state['storage']['hs']['last_hsid']++;
-    $hsid = $form_state['storage']['hs']['last_hsid'];
+    $_SESSION['storage']['hs']['last_hsid']++;
+    $hsid = $_SESSION['storage']['hs']['last_hsid'];

This is outlined in the main issue

+++ b/hierarchical_select.moduleundefined
@@ -1784,6 +1784,13 @@ function _hierarchical_select_hierarchy_generate($config, $selection, $required,
+
+        // Remove the level if it no longer has any children. This is possible
+        // if _hierarchical_select_apply_entity_settings removes the terms
+        // because they have no entities.
+        if (count($hierarchy->levels[$depth]) <= 1) {
+          unset($hierarchy->levels[$depth]);
+        }

Without this we get back empty selects. Would be better handled inside _hierarchical_select_apply_entity_settings perhaps.

+++ b/hierarchical_select.moduleundefined
@@ -1784,6 +1784,13 @@ function _hierarchical_select_hierarchy_generate($config, $selection, $required,
@@ -1856,6 +1863,9 @@ function _hierarchical_select_apply_entity_settings($level, $config) {

@@ -1856,6 +1863,9 @@ function _hierarchical_select_apply_entity_settings($level, $config) {
     $special_items['exclusive'] = array_keys(array_filter($config['special_items'], '_hierarchical_select_special_item_exclusive'));
     $special_items['none']      = array_keys(array_filter($config['special_items'], '_hierarchical_select_special_item_none'));
   }
+  else {
+    $special_items = array();
+  }
 
   // Only do something when the entity_count or the require_entity (or both)
   // settings are enabled.

if special_items isn't set we get a warning.

+++ b/modules/hs_taxonomy.moduleundefined
@@ -625,6 +625,18 @@ function hs_taxonomy_hierarchical_select_children($parent, $params) {
+  // If the term has no nodes associated with it, and it has no child terms,
+  // remove it.
+  foreach ($terms as $key => $term) {
+    $count = hs_taxonomy_hierarchical_select_entity_count($term->tid, array());
+    if ($count == 0) {
+      $children = taxonomy_get_children($term->tid);
+      if(empty($children)) {
+        unset($terms[$key]);
+      }
+    }
+  }

if ..entity_count() returns empty terms, get rid of them. otherwise the element has an empty (well label only) child select.

+++ b/modules/hs_taxonomy.moduleundefined
@@ -749,22 +761,18 @@ function hs_taxonomy_hierarchical_select_create_item($label, $parent, $params) {
-/*
-// TODO: port this to D7.
 function hs_taxonomy_hierarchical_select_entity_count($item, $params) {
   // Allow restricting entity count by node type.
-  if ($params['entity_count_for_node_type']) {
+  if (isset($params['entity_count_for_node_type'])) {
     return hs_taxonomy_term_count_nodes($item, $params['entity_count_for_node_type']);
   }

avoid a warning

+++ b/modules/hs_taxonomy.moduleundefined
@@ -749,22 +761,18 @@ function hs_taxonomy_hierarchical_select_create_item($label, $parent, $params) {
@@ -1040,8 +1048,7 @@ function _hs_taxonomy_hierarchical_select_get_tree($vid, $parent = 0, $depth = -

@@ -1040,8 +1048,7 @@ function _hs_taxonomy_hierarchical_select_get_tree($vid, $parent = 0, $depth = -
 }
 
 /**
- * Drupal core's taxonomy_term_count_nodes() is buggy. See
- * http://drupal.org/node/144969#comment-843000.
+ * Count the number of nodes assigned to a term, or its children terms.
  */
 function hs_taxonomy_term_count_nodes($tid, $type = 0) {
   static $count;
@@ -1055,14 +1062,19 @@ function hs_taxonomy_term_count_nodes($tid, $type = 0) {

@@ -1055,14 +1062,19 @@ function hs_taxonomy_term_count_nodes($tid, $type = 0) {
   }
 
   if (!isset($count[$type][$tid])) {
-    if (is_numeric($type)) {
-      // TODO Please convert this statement to the D7 database API syntax.
-      $count[$type][$tid] = db_query(db_rewrite_sql("SELECT COUNT(DISTINCT(n.nid)) AS count FROM {taxonomy_term_node} t INNER JOIN {node} n ON t.nid = n.nid WHERE n.status = 1 AND t.tid IN (%s)"), implode(',', $tids))->fetchField();
-    }
-    else {
-      // TODO Please convert this statement to the D7 database API syntax.
-      $count[$type][$tid] = db_query(db_rewrite_sql("SELECT COUNT(DISTINCT(n.nid)) AS count FROM {taxonomy_term_node} t INNER JOIN {node} n ON t.nid = n.nid WHERE n.status = 1 AND n.type = '%s' AND t.tid IN (%s)"), $type, implode(',', $tids))->fetchField();
+    $query = db_select('taxonomy_index', 't');
+    $query->innerJoin('node', 'n', '(t.nid = n.nid)');
+    $query->fields('n', array('nid'));
+    $query->condition('n.status', 1, '=');
+    $query->condition('t.tid', implode(',', $tids));
+    $query->distinct(TRUE);
+
+    if (!is_numeric($type)) {
+      $query->condition('n.type', $type, "=");
     }
+
+    $query->execute();
+    $count[$type][$tid] = $query->countQuery()->execute()->fetchField();
   }

Port to D7 and DBTNG.

samhassell’s picture

Status: Needs review » Needs work
FileSize
6.4 KB

Found some issues with the above patch stopping new taxonomy terms from appearing in the hierarchical select.

This new patch helps correct that, but it is still a bit buggy - top level terms with no children and no content will display an empty select box when selected.

samhassell’s picture

Looks like a lot of this patch made it into HS so I've stripped it down to just the hacky bit that needs work.

*edit* i was checking against code with the patch already applied :( -3 is still the latest.

samhassell’s picture

stefan.r’s picture

Status: Needs work » Closed (won't fix)

We don't use this piece of code anymore :)