Bug 9411 - Multiple uses of javascript eval on ajax responses in acq.js
Summary: Multiple uses of javascript eval on ajax responses in acq.js
Status: RESOLVED DUPLICATE of bug 4439
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Hugh Davenport
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-17 03:26 UTC by Hugh Davenport
Modified: 2018-06-28 18:34 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 9411 Fix javascript evals to use parseInt and parseFloat (3.26 KB, patch)
2013-01-17 03:49 UTC, Hugh Davenport
Details | Diff | Splinter Review
bug 9411 Fix javascript evals to use parseInt and parseFloat (3.25 KB, patch)
2013-01-17 04:43 UTC, Hugh Davenport
Details | Diff | Splinter Review
bug 4439 Change some helper files to use C4::Service (6.87 KB, patch)
2013-01-17 05:20 UTC, Hugh Davenport
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 ***