Plupload events support?

#plupload_settings will work only for simple values. AFAIK it won't allow you to pass functions through it.

This would be interesting feature, though. We implemented something similar for disqus.module. Maybe we can take same approach here? See http://drupalcode.org/project/disqus.git/commit/3ab6de3bff78d92f5b02b5c1... for more info.

Hm... not sure. Never tried to implement this feature. Isn't this last patch overriding default behaviour?

I actually didn't experience that Firefox behaviour with this patch. Maybe it is related to a specific event. Which events are you using? I am using Firefox 24.0 at Ubuntu 13.04.

We should probably implement autostart patch via events anyway (use #autostart and #autosubmit, create two functions in JS and put them in 'init' array in pre_render hook). If we take this approach we first need to get this in and then work on autosubmit after that.

+++ b/plupload.js
@@ -43,7 +43,19 @@
+      if (elementSettings['init'] || false) {
+        pluploadSettings.init = {};
+        for (var key in elementSettings['init']) {

I think that this feature would be easier to use if we would dedicate special Form API argument to it and then do some pre_render magic to translate it to JS settings. It would look something like this:

$form['upload'] = array(
  '#type' => 'plupload',
  '#event_callbacks' => array(
    'UploadProgress' => 'Drupal.myModule.uploadProgressCallback',
  ),
);

What do you think about that?

And of course... This also needs README.txt entry and both patches need documentation pages update as a follow-up.

1. +++ b/plupload.module
   @@ -258,7 +258,20 @@
   +  ¶
   +  if(isset($element['#autoupload'])) {
   +    $settings['init']['FilesAdded'] = 'Drupal.behaviors.plupload.FilesAdded';
   +  }
   +  if(isset($element['#autosubmit'])) {
   +    $settings['init']['UploadComplete'] = 'Drupal.behaviors.plupload.UploadComplete';
   +  }
   +  if(isset($element['#submit_element'])) {
   +    $settings['submit_element'] = $element['#submit_element'];
   +  }
   +  if(isset($element['#event_callbacks'])) {
   +    $settings['init'] = array_merge($settings['init'], $element['#event_callbacks']);
   +  }
   +  ¶
   

   I'd rather see this in PHP. pre_render hook is probably the right place to do that.

2. +++ b/plupload.js
   @@ -6,6 +6,24 @@
   +  ¶
   +  // Add Plupload events for autoupload and autosubmit
   +  FilesAdded:function (up, files) {
   +    setTimeout(this.start(), 100);
   +  },
   +  UploadComplete: function(up, files) {
   +    //if submit_element exists get that one (directly from Drupal.settings?):
   +    var submit_element = window.Drupal.settings.plupload["edit-files"].submit_element;
   +    if(submit_element) {
   +      $(submit_element).click();
   +    }
   +    // else just submit form you find
   +    else {
   +      var $form = $(".plupload-element").parents('form');
   +      $($form[0]).submit();
   +    }
   +  },
   +          ¶
      attach: function (context, settings) {
   

   This functions do not belong in Drupal.behaviours. We should probably move them to Drupal.plupload or something like this.

You are leaving a lot of trailing whitespace. Try to configure your IDE to auto-trim this.

Ah... stupid me... Sorry for that!

+  if(isset($element['#autoupload'])) {
+    $settings['init']['FilesAdded'] = 'Drupal.plupload.FilesAdded';
+  }
+  if(isset($element['#autosubmit'])) {
+    $settings['init']['UploadComplete'] = 'Drupal.plupload.UploadComplete';
+  }

Those should probably be empty() checks. Maybe also put #autosubmit inside #autoupload condition as it is kind of a dependency.

+  if(isset($element['#submit_element'])) {
+    $settings['submit_element'] = $element['#submit_element'];
+  }

We don't need that as it is already added above.

Otherwise looks very nice. Thank you for your great work!

Changing version as it needs to go to 7.x-2.x first, but it should be pretty easy to backport (cherry-pick will probably work w/o any problems).

Hm... you probably don't work against HEAD but 7.x-1.1. In 7.x-1.2 we added new option called "#submit_element". Pull latest code and you'll see what it is all about. It is good practice if you always create your patches against latest module code (i.e. HEAD).

+++ b/sites/all/modules/contrib/plupload/README.txt
@@ -36,6 +36,13 @@
+    'autoupload' => TRUE,
+    'autosubmit' => TRUE,
+    'submit_element' => '#id-of-my-submit-element',

My idea was to have those two as separate options (i.e. #autosubmit and #autoupload), not as a part of #plupload_settings. As they really do not map to settings 1-on-1.

Taking paragraph above into consideration we also don't need "submit_element" as it already exists. Check: http://drupalcode.org/project/plupload.git/commitdiff/6884b4a

No problem at all. I really appreciate your hard work. This will bring great new features to this module. I am planning to release new version when we commit this. And we are all making mistakes and learning every day anyway. :)

Yes... some whitespace was yours and some was in 7.x-1.1, but was already removed. It is very good habit to always enable auto-trim. This makes out code much cleaner.

I have just two little bits:

1. +++ b/sites/all/modules/plupload/plupload.js
   @@ -1,7 +1,23 @@
   +Drupal.plupload.uploadCompleteCallback = function(up, files) {
   +  var $this = $(".plupload-element");
   +  // If there is submit_element trigger it.
   +  var submit_element = window.Drupal.settings.plupload[$this.attr('id')].submit_element;
   

   This will work if there is only one plupload element on a form, but fail if there are more. Can we use "up" instead of "$this"?

2. +++ b/sites/all/modules/plupload/plupload.module
   @@ -265,11 +265,22 @@
   +    $settings['init'] = array_merge((array)$settings['init'], (array)$element['#event_callbacks']);
   +  }
    
   

   Why do we need this casts?

2) I put comment just above that line, thing is array_merge() expects arrays as type php manual and if our $settings['init'] is not set, has value NULL so implicitly we are casting it to an array. It just won't work with NULL, believe me, I tried, hehe. :)

I think it would be better to properly initialize $settings['init'] as an empty array if not initialized yet. It makes code much more readable and understandable.

The other one looks good.

Great! Committed to both 7.x-2.x and 7.x-1.x.

http://drupalcode.org/project/plupload.git/commit/969141e
http://drupalcode.org/project/plupload.git/commit/9236913

Thank you so much for your great work!

P.S.: Another suggestion. Consider using interdiffs when contributing. It is becoming a standard nowdays, specially if you contribute to core. More info: https://drupal.org/documentation/git/interdiff