Posted by zeropaper on February 22, 2009 at 7:34pm
| Project: | AHAH helper |
| Version: | 6.x-2.0 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
This is what i found in the module:
$settings = call_user_func_array('array_merge_recursive', $javascript['setting']);
drupal_json(array(
'status' => TRUE,
'data' => theme('status_messages') . drupal_render($form_item),
'settings' => array('ahah' => $settings['ahah']),
));It has a major inconvenient if the "data" passed had to use other javascript behaviors who were not set the first time (when the form is first rendered).
So I changed it to:
$settings = call_user_func_array('array_merge_recursive', $javascript['setting']);
drupal_json(array(
'status' => TRUE,
'data' => theme('status_messages') . drupal_render($form_item),
'settings' => $settings,
));And now my "newly loaded" table (with tabledrag javascript) works fine...
By the way... I don't know if it's a bug or a feature (and if i should post it here but (around line 150) i guess require_once($form_state['storage']['#ahah_helper']['#file']); should be replaced by require_once($form_state['storage']['#ahah_helper']['file']);...
Comments
#1
A patch for both bugs is attached. However, the file bug is also listed #368328: caching of built forms prevents form_state['values'] to be populated from $_POST with a patch for just that.
#2
Forgot to change status
#3
Anyone maintaining this?
#4
Not me. I just rolled a patch based upon the existing description.
#5
I am. But not frequently enough, I know. I've got so many stuff going on to balance between. And Hierarchical Select has about 5 times more uses than this module, so over the past few weeks, I've focused on that module. I'll make this module my focus the next few weeks!
This:
'settings' => array('ahah' => $settings['ahah']),is *by design*. Otherwise you end up with nasty things: new settings would override old settings, which can cause *a lot* of problems. I've had to fix this very bug (because the way it currently works, it's *not* a bug) in several modules because it caused problems with others.
AHAH in Drupal still sucks, I'm afraid, there isn't a solid/complete enough framework in core yet! :( This is one of those things that sucks.
#6
I confirm that restriction to just '#ahah' settings makes problems for other behaviors using Drupal.settings that can be changed after reload. tabledrag and date_popup elements are just two examples. If on the other hand, you experience states that there can be problems with other modules I would suggest at least to add configuration allowing to developer to choose setting branches that should be reloaded. I will send patch for this a bit later but will be glad to here feedback about this idea.
#7
Here is a patch implementing this configuration. Configured setting names are stored in regular Drupal variable and edited through administration settings form
#8
Sometimes drupal_render can add more javascript setting with calling drupal_add_js by other modules like cck, for example function theme_content_multiple_values() in content.module.
In this case we must render the form before fetching javascript settings, something like this:
function ahah_helper_render($form_item_to_render = FALSE) {
...
// Get the form item we want to render.
$form_item = _ahah_helper_get_form_item($form, $form_item_to_render);
$rendered = drupal_render($form_item);
// Get the JS settings so we can merge them.
$javascript = drupal_add_js(NULL, NULL, 'header');
$settings = call_user_func_array('array_merge_recursive', $javascript['setting']);
drupal_json(array(
'status' => TRUE,
'data' => theme('status_messages') . $rendered,
'settings' => $settings,
));