Community
Participate
Working Groups
Created attachment 187991 [details] screenshot of first prompt to download after trying to import non-zip M5 candidate from master, Win7, FF 3.6.13 I was trying import for the first time and didn't notice/read the "import a zip" title on the dialog. So I had selected a text file. When I clicked finish, I got a download prompt from FF (see attached screenshot). I cancelled the download prompt and clicked finish again. This time, the browse button animates briefly and then nothing happens. At this point, the import dialog will never work again (even for zip files) unless you reload all the javascript code. The silent failure for non-zips is disturbing, but the fact that zip downloads won't work again (even if you close/open the dialog) is much worse.
Created attachment 187992 [details] screenshot of different behavior on chrome On Chrome the behavior is different. If you pick a non-zip, it shows the filename with "fakepath" used as the path. The failure is similar to FF (it won't import, fails silently). BUT...the good news is that you can choose a zip file and it WILL work, the failure does not last for the life of the JS in the browser.
On IE 8.0.7600.16385, Win7 It tries to do the download of the non-zip. That fails, and you end up back in the import dialog. At that point the import dialog won't work until you refresh the JS code.
dojo FileUploader sometimes goes into a weird state when only a refresh helps :|
I modified the code in TransferServlet so it returns 400 (HttpServletResponse.SC_BAD_REQUEST) instead of 201 (HttpServletResponse.SC_CREATED). If the error is returned instead of OK status, the download is not triggered. The Import dialog is just closed.
this would be a good polish item for RC3. The symptoms are slightly different, but still confusing. On Chrome, it seemed like after a failed import, i started getting strange errors when trying to do other things, like delete a file. On Firefox, you get a 404 bad request error (which seems like the right response) but the UI doesn't report it. We should either not let the user do a non-zip, or else handle the error gracefully, or possibly even let the user upload a single non-zip file (I find myself wanting to do this all the time!)
sigh, I don't see an easy way to deal with this in the RC3 timeframe. (In reply to comment #5) > this would be a good polish item for RC3. > > We should either not let the user do a non-zip, The old dojox.form.fileuploader had a fileMask array. But the (new in dojo 1.6) dojox.form.uploader, which we are using, does not support a file mask. Apparently some HTML5 specs discuss a mask for inputs of file type, but it's not consistently implemented and therefore not supported in the dojox.form.uploader API > or else handle the error > gracefully, Both Chrome and FF are reporting a 400 (bad request) in the console. This seems like a reasonable thing for the server to do. Chrome also reports some additional errors (HTML Upload Error: Cannot read property 'value' of undefined) and additional errors reported by IFrame.js in the uploader (not sure why FF doesn't run that error handler, I put a breakpoint in the code and it was never hit.) I tried including onError handling in the ImportDialog (currently it hides the dialog, I added code to report the error in Orion) but that code was never hit in the debugger. I also found this in uploader.js onError: function(/* Object or String */evtObject){ // summary: // Fires on errors // //FIXME: Unsure of a standard form of error events }, In summary I'm having trouble hooking in any kind of error handler. > or possibly even let the user upload a single non-zip file (I find > myself wanting to do this all the time!) This would require server-side code. What I think we should do (in 0.4) is: - modify the server side to accept non-zips (new feature, usability) - consider allowing multiple file selection (new feature, usability) - consider getting rid of import dialog and instead just insert the dojox.form.filelist and uploader into the dom (this is just a presentation issue, we don't really need a dialog here, we could just open up a file selection input somewhere) - figure out the error reporting and how to ensure that server reported problems are exposed by our client rather than going to the console
generalizing title since previous comment discusses general enhancements.
*** Bug 367368 has been marked as a duplicate of this bug. ***
Any news on this one?
Defer to 0.5 would be my suggestion. This is a nice enhancement but there is an easy workaround of using a zip file.
I am working on a fix that will enable us to do the following: 1. Upload zip and non-zip files 2. Specify overwrite options 3. Specify whether or not the zip file needs to be extracted (the user might simply want an archive to be present in his project. I think this is not supported now) I will soon post a patch for review.
Created attachment 211864 [details] Patch for server components Patch contains necessary changes to support either a zip or non-zip file. It also makes unzipping optional, which means users can directly copy zip files from their local disk. The patch might need some clean-up. Another point is, the multipart request is parsed in a limited way only for this specific use case. If we think we might be working with multipart forms in other places too, we either have to look at a third-party parser or write a full-fledged parser.
Created attachment 211865 [details] Patch for client component A partial front end support is provided in this patch. It doesn't yet handle the errors well. On an error, the dialog is not dismissed and only the console logs the error status. This needs more work. But just about enough to test the previous patch. A question that comes to my mind is, do we want to support selection and importing of multiple files too? This would be a handy feature in my opinion.
Thanks for the patch Jay. I will review at least the server side of this change, and probably pull Anton in for the client because he has some ideas there. I'm assigning the bug to myself for now so I don't lose track of it.
Hi Jay, I'm reviewing your patch and trying to understand the changes. It looks like it now requires a multi-part POST message with various header supplied in the form (like the destination file name). This seems more complicated for clients.. our previous API supported the simple case of the POST body to be the exact contents of the file, and used HTTP headers to pass extra information and arguments. I seem to remember it being difficult for some clients like dojo xhr to build multi-part form POSTs. Do you think we can go back to a simpler approach without a multi-part body, or is there some limitation to this? Reviewing our old code I see we had a "raw" option to indicate the file should not be unzipped. it looks like you changed this to a multi-part segment called "extract". Is there a reason for doing it that way? I should have given you this reference before: http://wiki.eclipse.org/Orion/Server_API/Transfer_API To give some context, we essentially supported two different import mechanisms before: 1) A POST request is used to start a transfer, followed by a series of PUT calls to upload chunks at a time. 2) A single POST request with the entire file to be uploaded. I originally designed 1) because I thought this would be more scalable and allow progress during upload. But, it turns out dojo didn't support any way to do this so we added 2). So for now 2) is the most important one to handle and I would be ok if we didn't support approach 1 at all now (since we don't have any client that can use it today). All of our TransferTest suites fail with this change. If possible I would like to support the new cases while having the old ones continue working.
dojo xhr does not handle binary - the dojo io frame needs to be used for that, but to be frank is not well documented for programmatic use outside of the file upload widget. Anton (In reply to comment #15) > Hi Jay, > > I'm reviewing your patch and trying to understand the changes. It looks like it > now requires a multi-part POST message with various header supplied in the form > (like the destination file name). This seems more complicated for clients.. our > previous API supported the simple case of the POST body to be the exact > contents of the file, and used HTTP headers to pass extra information and > arguments. I seem to remember it being difficult for some clients like dojo xhr > to build multi-part form POSTs. Do you think we can go back to a simpler > approach without a multi-part body, or is there some limitation to this? > > Reviewing our old code I see we had a "raw" option to indicate the file should > not be unzipped. it looks like you changed this to a multi-part segment called > "extract". Is there a reason for doing it that way? > > I should have given you this reference before: > > http://wiki.eclipse.org/Orion/Server_API/Transfer_API > > To give some context, we essentially supported two different import mechanisms > before: > > 1) A POST request is used to start a transfer, followed by a series of PUT > calls to upload chunks at a time. > 2) A single POST request with the entire file to be uploaded. > > I originally designed 1) because I thought this would be more scalable and > allow progress during upload. But, it turns out dojo didn't support any way to > do this so we added 2). So for now 2) is the most important one to handle and I > would be ok if we didn't support approach 1 at all now (since we don't have any > client that can use it today). > > All of our TransferTest suites fail with this change. If possible I would like > to support the new cases while having the old ones continue working.
(In reply to comment #15) > Reviewing our old code I see we had a "raw" option to indicate the file should > not be unzipped. it looks like you changed this to a multi-part segment called > "extract". Is there a reason for doing it that way? > > I should have given you this reference before: > > http://wiki.eclipse.org/Orion/Server_API/Transfer_API There was no particular reason. But thanks for sharing the link. I will stick to what we already have. I am working on the patch.
I have fixed this. You can now import non-zip files. If the file is a zip we will still extract it. This was already supported on the server - I just had to tweak the headers sent from the client: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=0693279a7b4fc03bb08c689cdb9ea287f96f2127
thanks, John, you just made my life way easier for uploading new icons, etc.
(In reply to comment #6) > - figure out the error reporting and how to ensure that server reported problems > are exposed by our client rather than going to the console John, Jay, has the bullet above been covered by this bug?
(In reply to comment #20) > (In reply to comment #6) > > - figure out the error reporting and how to ensure that server reported problems > > are exposed by our client rather than going to the console > > John, Jay, has the bullet above been covered by this bug? No, the error case in question was when you attempted to import non-zip. This went away because we now allow that.
In that case I think it makes sense to file a separate bug for the error handling on the client side => bug 378456.
