---- Reported by nahuel.angelinetti@biblibre.com 2010-05-12 13:58:58 ---- There is some weird code in opac-search.pl and search.pl to parse some multivalued variable that broke index search in some cases. --- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:27 UTC --- This bug was previously known as _bug_ 4491 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=4491 Actual time not defined. Setting to 0.0 Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one.
Patch: http://lists.koha-community.org/pipermail/koha-patches/2010-May/011815.html I see the patch was pushed to rel_3_0 but the old code is still in HEAD. Does this fix need to be ported to HEAD?
Yep, this needs porting to master, if people want it in there
Created attachment 11110 [details] [review] Bug 4491: fix weird code in search scripts This change some code a bit strange, that generate strange effetcs. Now we use $cgi->param to get the values. Test: search at the opac and intranet using limit, etc.
Jonathan At first glance this looks good, but do you have an example of a search that provides bad results before this patch, and that is fixed by it please?
(In reply to comment #4) Chris, > At first glance this looks good, but do you have an example of a search that > provides bad results before this patch, and that is fixed by it please? There is no bug before the patch. It is purely esthetic :) It replaces the dumpy slit('\0') with a more "graceful" cgi->param (which is used everywhere into Koha).
Oh interesting Because it was reported as "here is some weird code in opac-search.pl and search.pl to parse some multivalued variable that broke index search in some cases." And then "This change some code a bit strange, that generate strange effetcs. Now we use $cgi->param to get the values." If its purely aesthetic, then it is definitely not major. If however Nahuel was right we can bump it back up to major.
I, too, am confused about what I'm supposed to testing here. Perhaps you could clarify what you meant by "that generate strange effects"?
(In reply to comment #6 and comment #7) Nahuel is not reachable currently. And I don't know if he remembers about this bug :) But when I accidentally found this bug, it seemed to me good to replace the dumpy split. If you want I can re-submit the patch by changing the comment to underline this point.
This patch breaks the OPAC search with the following message in the log: [Sat Aug 11 08:02:14 2012] [error] [client 192.168.56.1] [Sat Aug 11 08:02:13 2012] opac-search.pl: WARNING: query problem with (rk=(Title-cover,ext,r1="smith" or ti,ext,r2="smith" or Title-cover,phr,r3="smith" or wrdl,right-Truncation,r9="smith? " or wrdl,r9="smith")) and ZOOM error 10014 "CCL parsing error" (addinfo: "Search word expected") from diag-set 'ZOOM' at /home/jcamins/kohaclone/C4/Search.pm line 356., referer: http://192.168.56.57/cgi-bin/koha/opac-search.pl?limit= (my search was "smith") The staff client search works fine.
Created attachment 11740 [details] [review] Bug 4491: FIX buildQuery if called with an empty limit if @limit=('') buildQuery failed
Created attachment 11847 [details] [review] Bug 4491: fix weird code in search scripts This change some code a bit strange, that generate strange effetcs. Now we use $cgi->param to get the values. Test: search at the opac and intranet using limit, etc. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 11848 [details] [review] Bug 4491: FIX buildQuery if called with an empty limit if @limit=('') buildQuery failed Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> All searches that I tried (keyword, indexed, CCL, with limits, without, etc.) worked fine. There are warnings about uninitialized variables in the OPAC, but they exist on master as well and therefore should not block these patches.
> > Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> patches looks good, passing QA… mason@xen1:~/g/head$ koha-qa.pl -c 2 testing 2 commit(s) (applied to commit 0acebb8) * 65911df Bug 4491: FIX buildQuery if called with an empty limit C4/Search.pm * d8b1b52 Bug 4491: fix weird code in search scripts catalogue/search.pl opac/opac-search.pl * C4/Search.pm OK * catalogue/search.pl OK * opac/opac-search.pl OK
Patch pushed to master I think there's no need to push it to 3.8, (just in case we missed some side effect)
(In reply to comment #14) > Patch pushed to master > > I think there's no need to push it to 3.8, (just in case we missed some side > effect) Second patch (FIX buildQuery if called with an empty limit) is needed.