Bug 10956 - Series of OPAC searches can cause a browser crash
Summary: Series of OPAC searches can cause a browser crash
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Marc Véron
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-26 17:11 UTC by Owen Leonard
Modified: 2015-06-04 23:33 UTC (History)
4 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
Screenshot debugger after script not responding (81.85 KB, image/png)
2014-11-18 17:52 UTC, Marc Véron
Details
[HACK] Bug 10956 - Series of OPAC searches can cause a browser crash (1.35 KB, patch)
2014-11-18 19:31 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 10956 - Series of OPAC searches can cause a browser crash (1.36 KB, patch)
2014-11-19 09:26 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 10956 - Series of OPAC searches can cause a browser crash (2.23 KB, patch)
2014-11-19 10:43 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 10956 - Series of OPAC searches can cause a browser crash (2.34 KB, patch)
2014-11-25 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 10956 - Series of OPAC searches can cause a browser crash (2.56 KB, patch)
2014-11-25 13:31 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2013-09-26 17:11:09 UTC
In my tests this series of OPAC searches will result a browser crash (Firefox) or an endlessly loading tab (Chrome):

1. Perform a keyword search which will return a good number of results.
2. Click on a title in those results which will have a good number of 
   works by that author.
3. Click on the author link on the detail page to perform a search
   by author.
4. Click on a facet on the author search results page (I tested by 
   clicking on a library facet).
5. Click on a title in the refined search results.

I've tested this procedure with XSLT on and off, with OpacBrowseResults on and off, and with EnableOpacSearchHistory on and off. I get the same results in both themes.
Comment 1 Owen Leonard 2013-09-26 17:12:07 UTC
More info: The bug appears to affect master and 3.12.x but not 3.10.x. I couldn't reproduce the problem in the staff client following the same steps.
Comment 2 Mathieu Saby 2013-11-27 18:26:39 UTC
Don't you think this bug priority needs to be reevaluated?
Random browser crashes are a really nasty issue...

I suppose it is linked with the browser crashes I experienced each time I tried to test Jonathan Druart's patch made for making facets cancellable (Bug 10857)

Mathieu
Comment 3 Marc Véron 2014-11-18 17:30:26 UTC
I could reproduce endless loops as well, leading to a "Script not responding" message in Firefox.
What I did:
- Keyword search for "a"
- In Facets click an author with a lot of titles
- Click a detail page
Result: Wait cursor, then "Script not responding"

It seems to be related to a function jQuery.fn.higlight
It has a loop like
for (var i=0; i < node.childNodes.lenght; ++i)

in this loop, node.childNodes.lenght is really huge (sorry, could not take a screenshot)

After turning off syspref 'OpacHighlightedWords' I could not reproduce the endless loop again.
Comment 4 Marc Véron 2014-11-18 17:52:40 UTC
Created attachment 33662 [details]
Screenshot debugger after script not responding

Screenshot of debugger after getting "Script not responding" (Firefox)
Comment 5 Marc Véron 2014-11-18 19:31:04 UTC Comment hidden (obsolete)
Comment 6 Marc Véron 2014-11-18 21:21:21 UTC
New finding:

In facet, click several times on the author.
The link will get longer and longer and looks like the following (newlines by mv):

http://10.0.0.21/cgi-bin/koha/opac-search.pl?q=ccl=
kw%3Aa%20%20
and%20au%3ATwain%2C%20Mark%2C%201835-1910%20
and%20au%3ATwain%2C%20Mark%2C%201835-1910%20
and%20au%3ATwain%2C%20Mark%2C%201835-1910&sort_by=relevance&limit=au:Twain%2C%20Mark%2C%201835-1910

The highlighting function then will try to highlight the words Twain, Mark and 1835-1910 several times. 
Maybe this breaks the highlighting function.
Comment 7 Marc Véron 2014-11-18 21:54:38 UTC
Comment on attachment 33667 [details] [review]
[HACK] Bug 10956 - Series of OPAC searches can cause a browser crash

Obsoleted patch that helped to track down issue.
Comment 8 Marc Véron 2014-11-18 23:33:59 UTC
Comment on attachment 33667 [details] [review]
[HACK] Bug 10956 - Series of OPAC searches can cause a browser crash

Hmm, could not really find out what happens. 
At least this patch prevents the browser from freezing.
Comment 9 Marc Véron 2014-11-19 09:26:53 UTC Comment hidden (obsolete)
Comment 10 Marc Véron 2014-11-19 10:14:29 UTC
This fix is for the bootstrap theme.

Earlier fixes were vor the prog them (Bug 9419 and Bug 8646)
Comment 11 Marc Véron 2014-11-19 10:43:09 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2014-11-25 12:45:59 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2014-11-25 13:31:53 UTC
Created attachment 33903 [details] [review]
[PASSED QA] Bug 10956 - Series of OPAC searches can cause a browser crash

This patch fixes the issue for bootstrap. It was earlier fixed for the prog theme.

Solution: Prevent  jQuery.fn.highlight = function(pat) to be called with empty pat
by skipping empty values in highlight loop.

To test:
Without patch, do a search as in comment #1 odr #3
Result: Endless loop (Script not responding)

Appply patch:
No endless loop, page displays OK

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
To reproduce you need to search a string with 2 spaces.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I didn't manage to reproduce the problem, but I couldn't find
any regression either trying multiple searches.
Trusting Marc's and Jonathan's testing.
Comment 14 Jonathan Druart 2014-11-25 14:15:18 UTC
(In reply to Katrin Fischer from comment #13)
> Trusting Marc's and Jonathan's testing.

I confirm that my browser completely blown-up :)
I searched "    p.    21   "
This problem have already been fixed using the same way on the intranet and in the prod theme.
Comment 15 Tomás Cohen Arazi (tcohen) 2014-11-25 21:07:02 UTC
Patch pushed to master.

Thanks Marc!