DWR

File upload does not work in google chrome

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 3.0.RC1
  • Fix Version/s: 3.0.RC2
  • Component/s: engine
  • Description:
    Error message chrome throws when trying to use the inbuilt dwr file upload is:

    Uncaught TypeError: Cannot set property 'batch' of null
    Uncaught TypeError: Cannot set property 'batch' of null /dwr/engine.js (line 1486)

Attachments

  1. Text File
    file-upload.patch
    (4 kB)
    Jose Noheda
    20/May/09 9:32 AM
  2. Text File
    fileupload-all-amendment.patch
    (2 kB)
    James Crow
    08/Apr/09 7:14 AM
  3. Text File
    fileupload-all.patch
    (2 kB)
    Jose Noheda
    01/Apr/09 3:59 PM
  4. Text File
    fileupload.patch
    (2 kB)
    Jose Noheda
    17/Feb/09 9:57 AM
  5. Text File
    fileupload_engine.js.patch
    (3 kB)
    Jose Noheda
    18/Mar/09 6:36 PM

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Jose Noheda added a comment - 12/Feb/09 9:11 AM
Change line 1485 of engine.js from

batch.iframe = batch.document.getElementById(idname);

to

batch.iframe = batch.div.firstChild;

The form is posted to a new tab though
Show
Jose Noheda added a comment - 12/Feb/09 9:11 AM Change line 1485 of engine.js from batch.iframe = batch.document.getElementById(idname); to batch.iframe = batch.div.firstChild; The form is posted to a new tab though
Hide
Permalink
Joe Walker added a comment - 12/Feb/09 1:36 PM
I'm at a bit of a disadvantage with testing on chrome

So we're saying that only chrome fails to find the idname element? Do we know whay that is?

Jose - what do you mean by "posted to a new tab". Does that mean that chrome opens a new empty tab for the download? Is that down to the dwr.engine.openInDownload function?

Joe.
Show
Joe Walker added a comment - 12/Feb/09 1:36 PM I'm at a bit of a disadvantage with testing on chrome So we're saying that only chrome fails to find the idname element? Do we know whay that is? Jose - what do you mean by "posted to a new tab". Does that mean that chrome opens a new empty tab for the download? Is that down to the dwr.engine.openInDownload function? Joe.
Hide
Permalink
Jose Noheda added a comment - 13/Feb/09 7:31 AM
I meant that the submit, for some unknown reason, does not target the hidden iframe and hence opens a new tab/window. There's an added problem because the call response fails with an "Incomplete response from server" message (probably it has something to do with the new opened window). So, the file is posted correctly but if the remote method returns something that will be lost. I'll investigate a little bit more.
Show
Jose Noheda added a comment - 13/Feb/09 7:31 AM I meant that the submit, for some unknown reason, does not target the hidden iframe and hence opens a new tab/window. There's an added problem because the call response fails with an "Incomplete response from server" message (probably it has something to do with the new opened window). So, the file is posted correctly but if the remote method returns something that will be lost. I'll investigate a little bit more.
Hide
Permalink
Jose Noheda added a comment - 17/Feb/09 9:57 AM
Tested under IE7/8, FF3 and Chrome.

It stills fails under Opera but that may very well be resolved under another issue
Show
Jose Noheda added a comment - 17/Feb/09 9:57 AM Tested under IE7/8, FF3 and Chrome. It stills fails under Opera but that may very well be resolved under another issue
Hide
Permalink
Jose Noheda added a comment - 18/Mar/09 6:36 PM
Current code in HEAD fails on all browsers. This patch fixes it and adds (hopefully) chrome support
Show
Jose Noheda added a comment - 18/Mar/09 6:36 PM Current code in HEAD fails on all browsers. This patch fixes it and adds (hopefully) chrome support
Hide
Permalink
James Crow added a comment - 18/Mar/09 10:58 PM
I'll apply it to HEAD and let you know my findings!
Show
James Crow added a comment - 18/Mar/09 10:58 PM I'll apply it to HEAD and let you know my findings!
Hide
Permalink
James Crow added a comment - 19/Mar/09 6:08 PM
Jose, great fix, I can confirm that file upload now works with:
Google Chrome
IE 6
IE 7
Safari 4
Firefox 3

Think this closes quite a few other bugs that are in "Unscheduled" at the minute (I raised one about file upload in general being broken in HEAD)
Show
James Crow added a comment - 19/Mar/09 6:08 PM Jose, great fix, I can confirm that file upload now works with: Google Chrome IE 6 IE 7 Safari 4 Firefox 3 Think this closes quite a few other bugs that are in "Unscheduled" at the minute (I raised one about file upload in general being broken in HEAD)
Hide
Permalink
James Crow added a comment - 30/Mar/09 12:02 PM
Is this going to make it into HEAD? I applied to patch manually and it worked fine...
Show
James Crow added a comment - 30/Mar/09 12:02 PM Is this going to make it into HEAD? I applied to patch manually and it worked fine...
Hide
Permalink
Jose Noheda added a comment - 01/Apr/09 3:59 PM
This last one seems pretty good. Solves the Opera issues as well
Show
Jose Noheda added a comment - 01/Apr/09 3:59 PM This last one seems pretty good. Solves the Opera issues as well
Hide
Permalink
James Crow added a comment - 08/Apr/09 7:14 AM
The last patch you attached still works but introduces a bug in non-IE browsers - the batch.div is created but never appended to the document body, this just moves the creation of the batch.div into the IE-only section (which appends it to the document body)
Show
James Crow added a comment - 08/Apr/09 7:14 AM The last patch you attached still works but introduces a bug in non-IE browsers - the batch.div is created but never appended to the document body, this just moves the creation of the batch.div into the IE-only section (which appends it to the document body)
Hide
Permalink
James Crow added a comment - 08/Apr/09 7:16 AM
With reference to my last comment, the bug introduced was in the remove function, where batch.div existed but had no parentNode and was erroring:

      remove:function(batch) {
        // TODO: make it so that we don't need these if statements
        if (batch.div) {
            batch.div.parentNode.removeChild(batch.div);
        }
        .....
Show
James Crow added a comment - 08/Apr/09 7:16 AM With reference to my last comment, the bug introduced was in the remove function, where batch.div existed but had no parentNode and was erroring:       remove:function(batch) {         // TODO: make it so that we don't need these if statements         if (batch.div) {             batch.div.parentNode.removeChild(batch.div);         }         .....
Hide
Permalink
James Crow added a comment - 08/Apr/09 7:55 AM
Additionally the patch you applied causes a bug in chrome only, such that a file upload causes a new tab to open with a url of dwr/call/htmlcall/<method>. I have reverted back to the 2nd path (fileupload.patch) which for the minute seems to be the best working one.
Show
James Crow added a comment - 08/Apr/09 7:55 AM Additionally the patch you applied causes a bug in chrome only, such that a file upload causes a new tab to open with a url of dwr/call/htmlcall/<method>. I have reverted back to the 2nd path (fileupload.patch) which for the minute seems to be the best working one.
Hide
Permalink
David Marginian added a comment - 13/May/09 4:02 AM
There is still an outstanding issue here. I just tried the upload demo in the dwr.war in chrome and opera and it locked up both. Everything worked fine in IE and Firefox.
Show
David Marginian added a comment - 13/May/09 4:02 AM There is still an outstanding issue here. I just tried the upload demo in the dwr.war in chrome and opera and it locked up both. Everything worked fine in IE and Firefox.
Hide
Permalink
David Marginian added a comment - 13/May/09 4:15 AM
I take that back. I had actually reverted your patch :).

I just added it and re-tested. It works fine in all browsers. I also did not notice any of the issues described by James.

James, I have checked in Jose's latest patch if you are interested in grabbing and building from svn.
Show
David Marginian added a comment - 13/May/09 4:15 AM I take that back. I had actually reverted your patch :). I just added it and re-tested. It works fine in all browsers. I also did not notice any of the issues described by James. James, I have checked in Jose's latest patch if you are interested in grabbing and building from svn.
Hide
Permalink
James Crow added a comment - 13/May/09 9:29 AM
Still get the issue I reported before:

Non-IE bug introduced in the remove function, where batch.div existed but had no parentNode and was erroring:

      remove:function(batch) {
        // TODO: make it so that we don't need these if statements
        if (batch.div) {
            batch.div.parentNode.removeChild(batch.div);
        }

Additionally can't test the chrome bug right now but i'd imagine it still remains since it's the same patch I applied manually...
Show
James Crow added a comment - 13/May/09 9:29 AM Still get the issue I reported before: Non-IE bug introduced in the remove function, where batch.div existed but had no parentNode and was erroring:       remove:function(batch) {         // TODO: make it so that we don't need these if statements         if (batch.div) {             batch.div.parentNode.removeChild(batch.div);         } Additionally can't test the chrome bug right now but i'd imagine it still remains since it's the same patch I applied manually...
Hide
Permalink
Mike Wilson added a comment - 15/May/09 4:26 PM
Jose, I am equally impressed by, and frightened by, the solution to get this working in Chrome etc :-)
I tried out the patch back in April and, as we discussed on the mailing list, getting it to work in Chrome et al seems to depend on creating a totally unrelated DIV before triggering the iframe request. This make me feel that there is something lurking inside the iframe code that will bite us soon again...
I know Dojo has an iframe io provider, could you have a look at that and see if they manage without creating such a dummy element?
I'm really hoping that you can find an updated solution. That would probably also fix the Opera part of DWR-241.
Show
Mike Wilson added a comment - 15/May/09 4:26 PM Jose, I am equally impressed by, and frightened by, the solution to get this working in Chrome etc :-) I tried out the patch back in April and, as we discussed on the mailing list, getting it to work in Chrome et al seems to depend on creating a totally unrelated DIV before triggering the iframe request. This make me feel that there is something lurking inside the iframe code that will bite us soon again... I know Dojo has an iframe io provider, could you have a look at that and see if they manage without creating such a dummy element? I'm really hoping that you can find an updated solution. That would probably also fix the Opera part of DWR-241.
Hide
Permalink
Mike Wilson added a comment - 15/May/09 4:43 PM
BTW, the current file upload fails in Safari 2. I get an error dialog in the browser and an
  INFO UploadDownload.uploadFiles(null, null, "FFFFFF")
in the DWR log.
Again, Safari 2 seems to be handled by the Dojo code so could be interesting to look at.
Show
Mike Wilson added a comment - 15/May/09 4:43 PM BTW, the current file upload fails in Safari 2. I get an error dialog in the browser and an   INFO UploadDownload.uploadFiles(null, null, "FFFFFF") in the DWR log. Again, Safari 2 seems to be handled by the Dojo code so could be interesting to look at.
Hide
Permalink
Jose Noheda added a comment - 20/May/09 9:32 AM
Let's hope...
Show
Jose Noheda added a comment - 20/May/09 9:32 AM Let's hope...
Hide
Permalink
Mike Wilson added a comment - 24/May/09 2:00 AM
I'll take a look and see if I find some way to stop that spinning in Chrome 1 and Safari 3...
Show
Mike Wilson added a comment - 24/May/09 2:00 AM I'll take a look and see if I find some way to stop that spinning in Chrome 1 and Safari 3...
Hide
Permalink
Mike Wilson added a comment - 01/Jun/09 9:41 PM
I have now checked in a fix for getting a clean "iframe close" for Chrome 1 and Safari 3, which gets rid of the endless loading spinner.
Show
Mike Wilson added a comment - 01/Jun/09 9:41 PM I have now checked in a fix for getting a clean "iframe close" for Chrome 1 and Safari 3, which gets rid of the endless loading spinner.
Hide
Permalink
James Crow added a comment - 01/Jun/09 9:44 PM
Awesome, will give this a check tomorrow and let you know my findings!
Show
James Crow added a comment - 01/Jun/09 9:44 PM Awesome, will give this a check tomorrow and let you know my findings!
Hide
Permalink
Nicolas Bourdeau added a comment - 11/May/10 9:56 AM
I have the same exact bug right now with Chrome 5.0.375.29 beta (on linux) and DWR RC1 (build 116)
Is there a way to work around this bug while waiting for RC2 to come out ?
Is there planning on RC2 release ?
Show
Nicolas Bourdeau added a comment - 11/May/10 9:56 AM I have the same exact bug right now with Chrome 5.0.375.29 beta (on linux) and DWR RC1 (build 116) Is there a way to work around this bug while waiting for RC2 to come out ? Is there planning on RC2 release ?
Hide
Permalink
Mike Wilson added a comment - 14/May/10 12:59 PM
Get the latest dev build from http://ci.directwebremoting.org/bamboo/ to sort this out.
Hopefully we will get RC2 out soon.
Show
Mike Wilson added a comment - 14/May/10 12:59 PM Get the latest dev build from http://ci.directwebremoting.org/bamboo/ to sort this out. Hopefully we will get RC2 out soon.

People

Dates

  • Created:
    07/Jan/09 9:18 AM
    Updated:
    14/May/10 12:59 PM
    Resolved:
    01/Jun/09 9:41 PM
Atlassian JIRA (v4.1.2#531) | Report a problem | Request a feature | Contact administrators
Powered by a free Atlassian JIRA open source license for DWR. Try JIRA - bug tracking software for your team.