Bug 10652 - Ajaxfileupload.js failing in IE9 for local image upload
Summary: Ajaxfileupload.js failing in IE9 for local image upload
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-29 03:00 UTC by David Cook
Modified: 2015-06-04 23:30 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 10653 - Ajaxfileupload.js failing in IE9 for local image upload (1.88 KB, patch)
2013-07-29 03:08 UTC, David Cook
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10652 - Ajaxfileupload.js failing in IE9 for local image upload (2.12 KB, patch)
2013-08-05 14:35 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 10652 - Ajaxfileupload.js failing in IE9 for local image upload (2.15 KB, patch)
2013-09-06 11:33 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2013-07-29 03:00:42 UTC
Currently, when you try to upload a local cover image in IE9, Koha throws a Javascript error and reloads the "upload-cover-image.pl" page without uploading the image.

This appears to be the result of a change in the arguments available to the "createElement" Javascript function in IE9 (and probably IE9+). 

I imagine this has implications in other parts of Koha as well, including the MARC upload, which was the original motivator for the creation of ajaxfileupload.js, I believe. (The code was added by Galen in 2007 according to git blame, so he might know.)

In any case, I'm just going to introduce a try/catch which will cover all scenarios.
Comment 1 David Cook 2013-07-29 03:08:25 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2013-08-05 14:35:59 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2013-09-06 11:33:59 UTC
Created attachment 20822 [details] [review]
Bug 10652 - Ajaxfileupload.js failing in IE9 for local image upload

This patch adds a try/catch to the Javascript for adding the iframe
element. The try will work for IE8 and older versions, while newer
versions and other browsers will be covered by the check.

Test Plan:

1) Try uploading a local cover image in IE9
2) Notice that your page is reloaded with no result (except the
pop-up of a Javascript error, if you have error reporting on in
your browser).

3) Apply the patch

4) Trying uploading a local cover image again
5) Notice that it uploads correctly

6) Repeat steps 4 and 5 for IE8, IE7, and/or other modern browsers.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Tested in IE10 in IE10 "browser mode" and IE9 mode as well as in
Firefox. Uploads work correctly with book cover images and in MARC
uploads.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Also verified as working in Chrome.
Comment 4 Galen Charlton 2013-09-06 15:27:35 UTC
The AJAX file upload plugin was not written by me.  It was originally written by Øyvind Saltvik, although the current canonical fork appears to be http://www.phpletter.com/Our-Projects/AjaxFileUpload/.  The change log for the most recent version of this plugin claims to have fixed the IE9 issue.  For third-party code, I prefer that we stick with the stock version without modification unless upstream cannot or does not fix bugs that affect Koha.

To that end, does somebody have a moment to test the most recent version of the stock plugin?

There's another issue: I can find no statements whatsoever about the license of ajaxfileupload.js.  Mea culpa -- and that's actually an issue I care about more nowadays than my concern about not bundling modified third-party libraries.

It looks like the top upload plugin in the jQuery repository is jQuery File Upload
by Sebastian Tschan.  It's license is MIT.

Setting to in discussion to see if anybody has tuits to test an uploader plugin with a clear (and of course, compatible) license, but if those tuits don't arise soon, I'm not opposed to pushing David's patch as is, though I expect that it will be superseded.
Comment 5 Galen Charlton 2013-09-06 15:31:39 UTC
(In reply to Galen Charlton from comment #4)
> my concern about not bundling modified third-party
> libraries.

Or I should say, not modifying bundled third-party libraries.
Comment 6 Liz Rea 2014-07-01 22:17:19 UTC
I confirm that this fixes this issue, and also fixes another issue with syspref setting in IE9. I don't think I personally mind if this patch gets superseded in the future, I'd like to see it in.

Cheers,
Liz
Comment 7 Tomás Cohen Arazi 2014-07-03 13:20:01 UTC
Patch pushed to master.

Thanks David!

Note: new bug for updating Ajaxfileupload.js from upstream created (bug 12526)
Comment 8 Galen Charlton 2014-07-23 21:33:59 UTC
Pushed to 3.16.x for inclusion in 3.16.2.