I just started to play with the new Batch API and it doesnt seem to work correctly.

1. Batch operations dont work at all when JS is disabled/unavailable in Firefox. To verify this (and to proof that it is not my code) I downloaded the batch_example.module and tried again. It doesnt work with batch_example.module either. However it works as expected with IE7 and Opera9.

2. I successfully integrated a batch process into a page of my module, but on another page the following code (in a #submit handler) doesnt seem to invoke the batch function.

$operations = array();
$operations[] = array('_mymodule_function', array());
batch_set(array('operations' => $operations));

I only see the batch page (page without blocks and progress bar on the top), but function '_mymodule_function' is never called for some reason. As this seems strange, I will try to investigate why the code works on one page, but not on a second. However the problem in 1. is different and reproducible independently.

Comments

profix898’s picture

profix898 commented

Update: I found the reason why the batch function is not invoked in certain cases. The problem is that you cant have the function in a conditional include file (.inc). Say you have a foo.module, the batch function _foo_admin_batch() is defined in foo.admin.inc and you invoke the batch processing in _foo_admin_submit(). In this case you will get no errors because the function is initially available, but the function is actually never called. However if you move _foo_admin_batch() into the main foo.module (or include foo.admin.inc globally) it works as expected. I think we need to introduce an additional parameter to batch allowing to specify the file, where the batch function is defined (so that it can be included automatically). A global include is no solution IMO as it breaks the idea of splitting modules up.

Using a similar syntax as for hook_menu() this may look like this:

$operations = array();
$operations[] = array('_mymodule_function', array());
batch_set(array(
  'operations' => $operations,
  'file' => 'foo.admin.inc'
));

This would not allow to have operations in multiple files, but it would at least allow us to move batch functions into .inc files.

yched’s picture

yched commented

About .inc files : yes, the batch API got written and committed before the 'file' features in menu.inc, which made .inc files much more fashionable :-). The proposed solution could be a nice addition.

About batch processing not working when no JS : strange, it worked last time I checked (in FF as well). I'll try to investigate.

yched’s picture

yched commented

The proposed solution could be a nice addition.
For consistency, the 'file' should probably be a property of individual batch operations, rather then the whole batch, though.

profix898’s picture

profix898 commented

"the 'file' should probably be a property of individual batch operations"
I agree that would be the ideal solution! But it requires an API change (which will break existing code) while introducing an overall 'file' property would only add to the existing API. I'm all for per-operation includes, but it is for others to decide.

yched’s picture

yched commented

About no-JS mode : It does work. The problem might come from the way you test it : if you disable JS in your browser / FF tab and simply resubmit the batching form, the 'has-js' session cookie (which stores the fact that your JS is enabled) is still here.
If you disable JS, close your browser, re-open it, the non-JS mode is applied as expected.

This is more a side case, IMO, happening precisely because you're testing the thing. If you "really" intended to have JS disabled on your browser, it would have been disabled before the current (browser) session, and we wouldn't have detected it in the first place. I agree fixing this would be nice, but I'm not sure how...

BTW : the 'file' property is more a feature request than a bug, actually, and it should probably be submitted as such in a separate thread. The current behaviour is 'regular' wrt drupal file loading behaviour.

profix898’s picture

profix898 commented

If you disable JS, close your browser, re-open it, the non-JS mode is applied as expected.

No, it doesnt help to close the browser as the session cookie will usually still be there (depending on your browser settings). You must logout/login to replace/remove the cookie or kick it manually. I tend to test my code with/without JS by switching JS on/off, reloading the page and then submitting or playing with the page. This works with every JS feature in Drupal except for batch API. However (once you know that) it shouldnt be a problem in most cases.

the 'file' property is more a feature request than a bug

I cant agree with that. Its common practice in D6 to split off backend code into module.admin.inc or similar. And most batch API stuff will be used for backend operations (like mass node processing, etc.) as well. But without a 'file' property you would need to keep all batch functions in the main module or include them always and thats pretty bad IMO, because it nullifies any effort to get rid of unused code on 'normal' page loads.

Its not too bad to aggregate all batch operations (that are called at once) into one file IMO. And we can change it to a per-operation property in D7. The #submit/#validate property was per-form in D5 and is per-button in D6 ;)

yched’s picture

yched commented
Priority: Critical » Normal

Sorry, I still think it's a feature request (a valid one given the new abilities of menu.inc), not a bug. The two issues in this thread should be separated in different threads for the sake of discussion. Lowering priority from 'critical' to 'normal', anyway.

Besides, we can't handle this just as simply as in hook_menu definitions ('file' => 'mymodule.foo.inc'), because we have no way of knowing the module who defines the file, and thus the actual directory where the file resides.
I guess we could have something like :

$operations[] = array('_mymodule_function', array(),
                             'file' => array('module' => 'mymodule', 'filename' => 'mymodule.foo.inc'));
batch_set(array('operations' => $operations));

or

$operations[] = array('_mymodule_function', array(),
                             'file' => drupal_get_path('module', 'mymodule') .'/mymodule.foo.inc'));
batch_set(array('operations' => $operations));

The latter might actually be more efficient (saving the actual folder resolution on each iteration).

yched’s picture

yched commented

I submitted a patch for the 'file' stuff over at http://drupal.org/node/169079.

chx’s picture

chx commented
Status: Active » Closed (won't fix)

If you are doing such advanced thing as switiching on/off your JS while on a site you surely can just throw away the has_js cookie. This won't affect normal users.