Bug 10522 - Javascript error on acquisition pages: YAHOO is not defined
Summary: Javascript error on acquisition pages: YAHOO is not defined
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Owen Leonard
QA Contact:
URL:
Keywords:
Depends on: 9779
Blocks: 7180 10533
  Show dependency treegraph
 
Reported: 2013-07-01 12:09 UTC by Jonathan Druart
Modified: 2014-12-07 20:02 UTC (History)
1 user (show)

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


Attachments
Bug 10522 - Javascript error on acquisition pages: YAHOO is not defined (15.15 KB, patch)
2013-07-02 13:39 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 10522 - Javascript error on acquisition pages: YAHOO is not defined (15.38 KB, patch)
2013-07-03 08:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10522 - Javascript error on acquisition pages: YAHOO is not defined (15.43 KB, patch)
2013-07-07 07:55 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2013-07-01 12:09:35 UTC
git bisect tells me that bug 9779 introduces a javascript issue in some acquisition pages.
To reproduce:
  - open Firebug
  - go to acqui/addorderiso2709.pl

I don't know if the guilty code in js/acq.js can be removed.
Comment 1 Owen Leonard 2013-07-01 12:34:30 UTC
I have a patch in progress for this already. I'll try to get it submitted today.
Comment 2 Owen Leonard 2013-07-02 13:39:12 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2013-07-02 13:56:08 UTC
Don't you think it is worth to create, for instance a basketgroup.js, and to put this code in?
Comment 4 Owen Leonard 2013-07-02 14:46:31 UTC
(In reply to Jonathan Druart from comment #3)
> Don't you think it is worth to create, for instance a basketgroup.js, and to
> put this code in?

That would certainly work. It would also work to replace the YUI-dependent JS with something that doesn't require YUI. I'm offering a minimal patch because I'm not familiar with how the basketgroups drag-and-drop JS works and would prefer that someone more familiar with acquisitions take it up.
Comment 5 Jonathan Druart 2013-07-03 08:56:04 UTC Comment hidden (obsolete)
Comment 6 Chris Cormack 2013-07-07 07:55:25 UTC
Created attachment 19449 [details] [review]
Bug 10522 - Javascript error on acquisition pages: YAHOO is not defined

acq.js includes an immediately invoked function expression, which means
that it runs whether or not it is called. Because this function tries to
reference the YAHOO object, it triggers an error on pages which don't
include the main YAHOO assets. Only the basketgroups page uses this
function and YAHOO assets.

It's probably possible to make this a regular function, but I propose
simply wrapping it in a check for the YAHOO object so that it only
executes on pages where YAHOO exists--the basketgroups page.

To test, apply the patch, clear your browser cache, and test on both the
basketgroups page and at least one page which also includes acq.js
(addorderiso2709.pl, neworderempty.pl, aqbudgets.pl, suggestion.pl,
etc.) and confirm that the browser reports no JavaScript errors.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
$ git show HEAD -w
[...]
-
+if( typeof(YAHOO) === "object"){
     (function() {

     var Dom = YAHOO.util.Dom;
@@ -234,7 +234,7 @@ YAHOO.extend(DDList, YAHOO.util.DDProxy, {
         }
     });
     })();
-
+}

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 7 Galen Charlton 2013-07-08 15:10:27 UTC
Pushed to master.  Thanks, Owen!