Bug 10541 - Enable cross-browser AJAX in additem.js
Summary: Enable cross-browser AJAX in additem.js
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: David Cook
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 10607
  Show dependency treegraph
 
Reported: 2013-07-05 02:11 UTC by David Cook
Modified: 2014-12-07 20:02 UTC (History)
5 users (show)

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


Attachments
Bug 10541 - Enable cross-browser AJAX in additem.js (3.93 KB, patch)
2013-07-05 02:30 UTC, David Cook
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10541 - Enable cross-browser AJAX in additem.js (4.09 KB, patch)
2013-07-12 13:18 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 10541 - Enable cross-browser AJAX in additem.js (4.13 KB, patch)
2013-07-12 14:07 UTC, Kyle M Hall (khall)
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-05 02:11:48 UTC
Currently, the additem.js is using "indexOf" to search for a value in an array. While this works in Chrome, Firefox, and IE > 9, it fails miserably in IE 8 and 7 (which don't have the indexOf method). This means that users aren't able to add items using the acquisitions module!

Instead of using "indexOf", we should be using the jQuery function $.inArray. It was added in jQuery v1.2 (3.8.0 uses v1.3.2 so even our oldest supported release can use this method). It's perfectly cross-browser compatible...works in Chrome, Firefox, and every version of IE that I've tried (i.e. 7, 8, 9).

Patch and test plan to follow...
Comment 1 David Cook 2013-07-05 02:30:58 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2013-07-12 13:18:39 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall (khall) 2013-07-12 14:07:27 UTC
Created attachment 19594 [details] [review]
Bug 10541 - Enable cross-browser AJAX in additem.js

Currently, the additem.js is using "indexOf" to search
for a value in an array. While this works in Chrome, Firefox,
and IE > 9, it fails miserably in IE 8 and 7 (which don't have
the indexOf method). This means that users aren't able to add
items using the acquisitions module!

Instead of using "indexOf", we should be using the jQuery function
$.inArray. It was added in jQuery v1.2 (3.8.0 uses v1.3.2 so even
our oldest supported release can use this method). It's perfectly
cross-browser compatible...works in Chrome, Firefox, and every
version of IE that I've tried (i.e. 7, 8, 9).

Test Plan:

Before applying patch:

0) Switch to Internet Explorer 7, or 8, or 9.

If you're using IE 9, you'll need to change the Document Mode to
IE7 standards or IE8 standards.

You can do this by opening Internet Explorer 9, pressing F12 (or
clicking on the gear in the top right corner and choosing
F12 Developer Tools), and then clicking on "Document Mode" on the
top toolbar. There, you can change to IE7 or IE8 standards.

N.B. This is not always a perfect emulation in every case, but this
time it does show you the bug.

1) Set the system preference AcqCreateItem to "receiving an order"
2) Go to Acquisitions
3) Either:
    a) Receive a shipment for a basket with items
    b) Create a new basket, create an order, close the basket, and
       then do 3a)
4) In the "Item" fieldset, fill out some fields such as barcode,
   Date acquiried, Public note, etc.
5) Click "Add" at the bottom of the fieldset
6) Note that while the item may have been added, the "Item" fieldset
is not being shown again. You may also notice a Javascript error
appearing in a pop-up window or you might see a yellow warning flag
on the bottom status bar.

APPLY THE PATCH

7) Do a full refresh of the page (hold down shift and press the refresh
button on the browser next to the address bar), and try adding items
again.
8) Note that you receive no warnings and that items are added correctly
as they would be in Firefox or Chrome.

OPTIONALLY

9) To be sure that I haven't broken anything, go through the same steps
in IE9 (with IE9 standards) or Chrome or Firefox. Everything should be
working.

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

Tested in IE10 in IE7 mode and IE9 mode. Also tested in Firefox.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passes koha-qa.pl, works as advertised.
Comment 4 Katrin Fischer 2013-07-14 07:30:33 UTC
I have run into this in training as well and glad to see a patch for it as this has been quite a blocker bug for the library. There are also IE problems in serials and other modules - maybe we shold fix all IndexOf appearances?
Comment 5 Galen Charlton 2013-07-17 16:11:02 UTC
(In reply to Katrin Fischer from comment #4)
> I have run into this in training as well and glad to see a patch for it as
> this has been quite a blocker bug for the library. There are also IE
> problems in serials and other modules - maybe we shold fix all IndexOf
> appearances?

Indeed.  I have opened bug 10607 for tracking this.
Comment 6 Galen Charlton 2013-07-17 16:39:09 UTC
Pushed to master.  Thanks, David!
Comment 7 Tomás Cohen Arazi (tcohen) 2013-08-13 16:47:47 UTC
This patch has been pushed to 3.12.x, will be in 3.12.4.

Thanks David!
Comment 8 Bernardo Gonzalez Kriegel 2013-09-13 21:20:52 UTC
Pushed to 3.10.x, will be in 3.10.11
Comment 9 Chris Hall 2013-09-15 05:12:45 UTC
Pushed to 3.8.x, will be in 3.8.18

Awesome commit message :)