taxonomy_manager_update_term_data_form() tries to pass undeclared $form_state to form builders

+1, same here.

- Pressflow 6.20.97
- jQuery update 6.x-2.0-alpha1 (jq 1.3.2)
- Taxonomy Manager 6.x-2.2 release
- XMLsitemap 6.x-2.x-dev (2011-Feb-09)

Modules enabled:
XML Sitemap
XML Sitemap custom
XML Sitemap engines
XML Sitemap node

Error:

drupal: https://mysite.com|1234567890|php|1.2.3.4|https://mysite.com/admin/content/taxonomy_manager/termdata/3/99/true?form_build_id=form-f34a12f71e5740eaa47c623b538d4d42&form_id=taxonomy_manager_form|https://mysite.com/admin/content/taxonomy_manager/voc/3|1||Recoverable fatal error: Argument 2 passed to xmlsitemap_node_form_alter() must be an array, null given in xmlsitemap_node_form_alter() (line 161 of /my/path/to/drupal/sites/all/modules/xmlsitemap/xmlsitemap_node/xmlsitemap_node.module).

This also shows up in the Firebug console.

Not quite sure why, just chiming in here. May be more of a taxonomy_manager issue than an xmlsitemap issue.

More details:

- It's related to hook_form_alter, and the taxonomy manager module not passing in the proper $form_state.
- Disabling jquery_update has no impact on fixing it. Same problem happens with stock jQuery.
- It's definitely not an XMLsitemap issue. I wrapped the inside of xmlsitemap_node_form_alter function in this:

  if ($form_id != 'taxonomy_manager_form') {
  }

... and then the same thing happened, but with the search_theme_form. So this really needs to move to the taxonomy_manager issue queue.

Nope, I'm wrong again. It is an xmlsitemap_node.module issue. It should have been obvious :)

OK, so here's xmlsitemap_node_form_alter:

function xmlsitemap_node_form_alter(array &$form, array &$form_state, $form_id) {

This function is typecasting the inbound variables, which is not correct according to the hook_form_alter API. The fix is simple, just change it to:

function xmlsitemap_node_form_alter(&$form, &$form_state, $form_id) {

Patch attached against DRUPAL-6--2.

Doh, forgot to change status

Yes, but both of those variables *should* be arrays, so the type-hinting should already be correct.

I'm not disagreeing, I'm just saying that it breaks upstream modules that may not properly be passing in variables. In the case of taxonomy manager, it's sending in NULL for the form_state.

I also think typecasting is a good thing, but I think that's a different issue than my original point: it is not typecasted in the published API so it should not be typecasted here. I grepped the module tree for my production site and not one other module forcibly typecasts a hook_form_alter, including widely used modules such as cck, views, ctools, feeds, wysiwyg, flag, filefield, apachesolr, context, pathauto, etc. This is also true for all of the Drupal core modules (locale, forum, taxonomy, comment, node, etc.) So implementing it in that method here is going against the tide.

Will there be buggy modules? Sure, but if you really wanted to point out coding mistakes in other modules perhaps that would be more appropriate for the coder module (do a hook_form_alter without typecasting, then do an is_array per #7, and issue a report on modules that aren't in compliance.) Otherwise I think you run the risk of trying to prove a point here by silently breaking other modules.

Or we could just spend the time to fix it in the Taxonomy Manager code. Not providing an array for $form_state would cause other major problems. XML sitemap is the only want that forces a fatal error because of it. And for good reason.

Patch attached for taxonomy_manager.

Can someone please confirm that the only line added for this patch is
+ $form_state = array();

we are patching manually and see several - by blank lines and + by text already present.
Thanks

Sorry we are using version 6.x-2.2 as this is a production site but also got the log errors noted above.

+1 sub.

same problem

patch seems working fine, only

$form_state = array();

added

after patching, got this error:

Unknown error occurred while writing to file sites/default/files/xmlsitemap/NXhscRe0440PFpI5dSznEVgmauL25KojD7u4e9aZwOM/1.xml. v souboru H:\Zend\Apache2\htdocs\web.cz\sites\all\modules\xmlsitemap\xmlsitemap.generate.inc on line 158.

great. works fine for me :) thanks alot

Here's a re-roll of #10 that might apply more cleanly for some of you. It's the same thing, just the one line change.

Please test so we can go RTBC and have this added. I'd like to not have to maintain separate patches for production whenever possible. Thanks!

This is good. Tested and working correctly.

So...did this ever get committed to Taxonomy Manager? I still have the bug popping up with the newest versions of Taxonomy Manager and XML Sitemap.

Oh, and I can also confirm that the patch in #17 applied and fixed the issue for me.

I sent a direct message to mh86 who appears to be the most active maintainer. Hopefully we'll see a commit for this one.

Committed patch from #17 to 6.x-2.x-dev. Thanks to everyone involved in this issue!

Awesome, thank you!