When we use upload.js in imagefield for example, we can't upload more than one file at the same time, since Drupal uses one iframe for all uploads, so only one completes.
This patch changes drupal.js and upload.js so that each upload gets it's own iframe, so we can upload as many files as we want in parallel.

Also, in the patch are some fixes so that everything works with Opera.
I moved the iframehandler to it's own function, so it works with Opera. When it was part of the bigger function, Opera fired it straight away, instead of only oncomplete.

Review and tell me what you think,
Bojan

CommentFileSizeAuthor
drupal_upload.patch8.78 KBbojanz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Steven’s picture

Steven CreditAttribution: Steven commented
Status: Needs review » Needs work

I like the idea of this patch, but just looking through the code it doesn't appear quite right:

(removed '-' lines)

+      if(typeof(handlers) != 'object') {
+        action = button.form.action;
+        target = button.form.target;
+        handlers = new Array();
+      }
+      
+      Drupal.createIframe(base, target, action);

handlers, button and action are not declared, and the latter are only initialized if 'handlers' is undefined. This is the case if handlers == object, the code will pass undefined values into Drupal.createIframe for target/action. This is not right.

If this code still works, that means that the if-check always succeeds and is in fact redundant.

Also, code like this is silly:

+    $('div', div).each(function() {
+      if (('#'+ this.id) == hide) {
+        this.style.display = style;
+      }
+    });

jQuery can select by id immediately, there is no need to iterate like this.

bojanz’s picture

bojanz CreditAttribution: bojanz commented

I must admit I am not a JS expert, so anyone is free to improve the code.

The code doesn't look right maybe, but it works.

if(typeof(handlers) != 'object') {

This condition is true for the first time we upload, and false for every other time we upload (an imagefield for example).
So, if handlers is not set, we set the required data, and it persists to be used in other requests.
Without that code, action gets rewritten in the second upload with the wrong value (leading to imagefield/js).

As, for the second code you listed, Drupal does it this way, this is from the vanilla upload.js

// Note: workaround because jQuery's #id selector does not work outside of 'document'
    // Should be: $(this.hide, div).hide();
    var hide = this.hide;
    $('div', div).each(function() {
       if (('#'+ this.id) == hide) {
         style = this.style.display;
         this.style.display = 'none';
       }
     });

This line:

$(this.hide, div).fadeIn('slow');

Was making problems in Opera (didn't work), so I replaced it with code in style with the code 5 lines above, the iteration:

('div', div).each(function() {
      if (('#'+ this.id) == hide) {
        this.style.display = style;
      }
    });

So we get the same effect, but with opera working.

Feel free to correct the code if you know how, this is the best I can do.

p_palmer’s picture

p_palmer CreditAttribution: p_palmer commented

Something slightly wrong here....

How to recreate: Use FF and Web developer 1.1.4 extension, then Use Ctrl-Shift-F to Display Element Information...

I am not using multiple images per my particular content type.

Each time a user selects 'upload', the current preview image will be overwritten with the new - fine.

However, there is an extraneous "div" and "div #field-image-attach-wrapper" added when there should not be.

Web developer output...
html .js > body .sidebars > div #wrapper > div #container .clear-block > div #center > div #squeeze > div .right-corner > div .left-corner > form #node-form > div > div .node-form > div .standard > div #field-image-attach-wrapper > div > div #field-image-attach-wrapper > fieldset .collapsible > div .fieldset-wrapper > div .imagefield-edit-image-row clear-block

This section... div .standard > div #field-image-attach-wrapper > div > div #field-image-attach-wrapper > ...
should read... div .standard > div #field-image-attach-wrapper > ...

When a new image is uploaded, the markup goes away, however, it is quite important that this feature not happen if at all possible.

Can anyone else spot this bug?

Thanks in advance

Pancho’s picture

Pancho
Location UTC+2 🇪🇺 EU
CreditAttribution: Pancho commented
Title: Parallel uploads and Opera support » Fix parallel uploads and Opera support

Another bug that IMHO should be fixed in D6.

dpearcefl’s picture

dpearcefl CreditAttribution: dpearcefl commented

Is this still a need in D6? If so, who wants to supply a patch?

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.