ProgerXP / FileDrop

Self-contained cross-browser pure JavaScript class for Drag & Drop and AJAX (multi) file upload.
filedropjs.org
The Unlicense
264 stars 61 forks source link

Iframe callback is not run in IE9 #37

Open oytuntez opened 9 years ago

oytuntez commented 9 years ago

1) Demo page does not work on IE9, because the callback script is not run. When you run the callback yourself (window.top.fd_1234 type of calls) by copying from the server response, it works well.

I have been dealing with this problem since yesterday, the problem is:

2) This specific iFrame does not have top or parent context. The iFrame created thinks it is a standalone frame and does not have a parent window, nor has a frameElement. So fd_1234 is undefined because window.top = window and window.parent = window, right side being the iFrame's own window context.

I encountered the problem on IE9 so far, modern browsers seem to be working fine when options.iframe.force = true. I still couldn't come up with a solution, but it already cost me hours and hours...

Any ideas?

One possible explanation may be: http://msdn.microsoft.com/en-us/library/gg622929(v=vs.85).aspx How I reached the MSDN article: http://stackoverflow.com/questions/5514973/javascript-code-in-iframes-in-ie9-not-working

oytuntez commented 9 years ago

I retried the demo page on IE9.

ProgerXP commented 9 years ago

This is strange, I have tested all versions before releasing FileDrop v2 and it used to work. Now I can't even verify this because I can't download IE9 - it's not existing for XP and MS downloads gives me IE11 for Win7 with no choice.

Generally, IE9 share must be extremely low (<2%) if we believe http://www.w3schools.com/browsers/browsers_explorer.asp. I think it's best to just let it be especially because you say that it "sometimes works, and sometimes not" - if it's some random bug you can spend days looking for a solution.

oytuntez commented 9 years ago

I don't believe in anything w3schools says. I got a few visitors yesterday with IE8 in Arabic locale. My inbox was alarming with IE8-related error reports :)

The problem here was about how IE9 and FileDrop was using iframes... First, see this line: https://github.com/ProgerXP/FileDrop/blob/master/filedrop.js#L1139

When you move (or remove or else...) an iframe in IE9, it destroys any context that iframe used to have, such as window.top. resetForm method does exactly this :) It resets the iframe and the form.

To solve this, I removed the line 1139 and moved resetForm call after the line 1116. This solved everything, however, with a drawback: you can't use multiple iframes now. You need to wait until the first iframe finishes its job, which kind of made it impossible to select a file after a file and after a file without waiting for them to be processed.

ProgerXP commented 9 years ago

I got a few visitors yesterday with IE8 in Arabic locale. My inbox was alarming with IE8-related error reports :)

You're kidding, what OS do they use? Vista? I don't want to believe it :(

resetForm method does exactly this :)

Problem is that if you remove this method or set recreateInput to false you're preventing user from uploading the same file twice in a row.

Argh, IE has been driving me crazy and with v2 I was sure I have got rid of all of it quirks. Anyway, I might be willing to look into it some time later. How did you obtain IE9? Are you using Vista (on a VM of course)? The entire setup is going to be very fancy.

oytuntez commented 9 years ago

I think I currently have all VMs from https://modern.ie When I'm bored, I just load one of them to hear Windows loading sound... :+1:

I'm not removing resetSubmit completely, I only moved it to the callback you define several lines earlier, right after this line: global.callAllOfObject(self, 'iframeDone', resp) So it still resets the form, only after the callback from the iframe is loaded and run.

josephj commented 9 years ago

I also got this issue on IE9 as well.

@oytuntez,

Thanks man, you saved my day.

@ProgerXP,

For VMs, I prefer to use ievms which just need 1 command to install everything for me. https://github.com/xdissent/ievms And after that you can use iectrl to reinstall the expired ones. https://github.com/xdissent/iectrl

oytuntez commented 9 years ago

No problem, @josephj!

DiegoYungh commented 9 years ago

So is the proposed change from @josephj a resolution or will it break code ? Did you got any more errors after changing this @josephj ?

Also @ProgerXP what about the proposed change ?

oytuntez commented 9 years ago

@DiegoYungh, We have been using the solution I proposed above for a while, no errors reported in any browser so far. We are using Bugsnag to automatically collect errors from our clients.

The solution has a drawback, though, quoting from myself:

To solve this, I removed the line 1139 and moved resetForm call after the line 1116. This solved everything, however, with a drawback: you can't use multiple iframes now. You need to wait until the first iframe finishes its job, which kind of made it impossible to select a file after a file and after a file without waiting for them to be processed.

DiegoYungh commented 9 years ago

@oytuntez, then picking one file at a time and only after processing, doesn't look so bad. Should then disable the browse button while you can't select another.

ProgerXP commented 9 years ago

@oytuntez @DiegoYungh Sorry guys, I realize it's a huge delay but I'm tied up in other activities and can't give it enough attention for now. Please use the suggested patches if it works for you meanwhile.

jauco commented 9 years ago

@ProgerXP could you highlight this bug at the start of the readme? The readme starts with "supports ie6". Mentioning that ie <= 9 has this known bug would probably save people from having to discover it themselves. The workaround will be enough for some people.

makronetz-patricia commented 9 years ago

Hi,

I have tried @oytuntez's workaround, but the "iframeDone" event is never called on my side in IE 9. It is used in a symfony2 project.

    var options = {
        iframe: {
            url: '{{ path('landlord_object_read_file') }}',
            force: true
        }
    }
    var zone = new FileDrop('dropzone', options)

    zone.event('send', function (files) {
        files.each(function (file) {
            file.event('done', function (xhr) {
                alert(xhr.responseText)
            })

            file.sendTo('{{ path('landlord_object_read_file') }}')
        })
    })

What could the problem be? Thank you.

jhubert commented 9 years ago

@oytuntez Thank you for the fix suggestion. That should help me track this down.

@ProgerXP Most of the people we sell to use IE9 on Windows 7. Enterprise IT teams at major US corporations. Pity them? yes. But I still have to support them. :neckbeard:

Fwiw, I spent the last two hours struggling with this thinking it was my fault because the site says it works in IE6+. It wasn't until I noticed that IE9 was missing from the below image on the demo page that I started to wonder if it was the libraries fault.

image

jhubert commented 9 years ago

@makronetz-patricia make sure you have the iframeDone event callback in your code:

// Successful iframe upload uses separate mechanism
// from proper AJAX upload hence another listener:
zone.event('iframeDone', function (xhr) {
  console.log(xhr);
  alert(xhr.responseText);
})