Bug 9411

Summary: Multiple uses of javascript eval on ajax responses in acq.js
Product: Koha Reporter: Hugh Davenport <hugh>
Component: AcquisitionsAssignee: Hugh Davenport <hugh>
Status: RESOLVED DUPLICATE QA Contact:
Severity: major    
Priority: P5 - low CC: chris, jonathan.druart, kyle, veron
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: bug 9411 Fix javascript evals to use parseInt and parseFloat
bug 9411 Fix javascript evals to use parseInt and parseFloat
bug 4439 Change some helper files to use C4::Service

Description Hugh Davenport 2013-01-17 03:26:45 UTC
hugh@hugh-desktop:~/code/koha (master)$ cat koha-tmpl/intranet-tmpl/prog/en/js/acq.js | grep eval
    // rc =  eval ( xmlhttp.responseText );
            actTotal = eval ( xmlhttp.responseText );
    var result = eval ( xmlhttp.responseText );
    var result = eval ( xmlhttp.responseText );
hugh@hugh-desktop:~/code/koha (master)$
Comment 1 Hugh Davenport 2013-01-17 03:27:50 UTC
A brief look tells me that it is checking for numerical types, which would be better suited by parseInt and parseFloat
Comment 2 Hugh Davenport 2013-01-17 03:30:33 UTC
fixing
Comment 3 Hugh Davenport 2013-01-17 03:49:49 UTC Comment hidden (obsolete)
Comment 4 Hugh Davenport 2013-01-17 04:43:42 UTC Comment hidden (obsolete)
Comment 5 Hugh Davenport 2013-01-17 05:20:12 UTC
Created attachment 14652 [details] [review]
bug 4439 Change some helper files to use C4::Service

Depends on bug 4437 to avoid conflicts

Depends on bug 9412 for a new feature used

Also fixes bug 9411

Signed-off-by: Hugh Davenport <hugh@davenport.net.nz>
Comment 6 Kyle M Hall 2013-01-18 14:56:27 UTC
Hugh, just a note, the patches you are submitting appear to each have a sign-off line from yourself.
Comment 7 Chris Cormack 2013-01-18 15:05:10 UTC
Yep, that's quite common in a lot of projects, you'll notice Galen does it also. It means they have tested it themselves. Still needs an external sign off of course. 
In fact just the other day Galen remarked something like "when did we stop adding a sign off to our patches?"
Comment 8 Kyle M Hall 2013-02-15 12:34:58 UTC
Hugh, can you add a test plan to the patches comments?
Comment 9 Owen Leonard 2013-03-14 13:33:18 UTC
Failing QA pending the addition of a test plan.
Comment 10 Hugh Davenport 2013-07-05 21:13:30 UTC
Hey Kyle/Owen,

What would the format of a test plan be?

Cheers,

Hugh
Comment 11 Kyle M Hall 2013-07-08 13:26:41 UTC
Since the patch in question doesn't change the behavior of Koha ( at least in theory ) it would be as simple as 
1) Perform some action that tests the code to be modified
2) Apply this patch
3) Perform the same action, ensure everything still works the same.

What that the action or actions are, is up to you.
Comment 12 Jonathan Druart 2015-02-16 12:36:00 UTC
Hugh, Any chances to see this resubmitted?
Comment 13 Marc VĂ©ron 2015-12-24 13:50:18 UTC
Is this bug still valid? (Does not apply).
Comment 14 Jonathan Druart 2018-06-28 18:34:43 UTC

*** This bug has been marked as a duplicate of bug 4439 ***