Bug 34005 - Toggling the search term highlighting is not always working in the bibliographic record details page
Summary: Toggling the search term highlighting is not always working in the bibliograp...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Andreas Roussos
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-06-14 09:01 UTC by Andreas Roussos
Modified: 2023-12-28 20:46 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02,22.11.08


Attachments
Bug 34005: Move button toggling code outside JS for loop (2.33 KB, patch)
2023-06-14 14:38 UTC, Andreas Roussos
Details | Diff | Splinter Review
Bug 34005: Move button toggling code outside JS for loop (2.38 KB, patch)
2023-06-14 14:47 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 34005: Move button toggling code outside JS for loop (2.45 KB, patch)
2023-06-26 11:17 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Roussos 2023-06-14 09:01:35 UTC
To reproduce, make sure you have the OpacHighlightedWords System
Preference set to "Highlight", then perform a search in the OPAC.

NOTE: the number of keywords you search for is important, as this
bug only manifests when an odd number of search terms is used!
Therefore, try searching for a single keyword such as "history".

Now view the details page for one of the bibliographic records in
the results. On the right there should be an "Unhighlight" button
which, when clicked, will remove the search term highlighting and
change the button text to "Highlight". Clicking on "Highlight"
should re-enable the highlighting , but now the button text will
not toggle back to "Unhighlight" and further clicks on it have no
effect.

The relevant commit is 9cb89b46397 from v20.11.00, where the code
responsible for toggling the Unhighlight / Highlight button was
moved inside the for loop that cycles through each term in the
array of search terms. This explains why the toggling works only
when an even number of search terms is being used.
Comment 1 Andreas Roussos 2023-06-14 14:38:12 UTC
Created attachment 152337 [details] [review]
Bug 34005: Move button toggling code outside JS for loop

When OpacHighlightedWords is set to "Highlight", if you search
in the OPAC and the number of keywords is odd (1, 3, 5, etc.),
then the "Unhighlight" / "Highlight" button will stop working
after a couple of clicks. The relevant commit is 9cb89b46397
from v20.11.00, where the code responsible for toggling the
"Unhighlight" / "Highlight" button was moved inside the JS
for loop that cycles through each term in the array of search
terms. This explains why the toggling only works when an even
number of search terms is being used.

This patch fixes that, by placing the button toggling code in
its original position (outside the JavaScript for loop of the
highlightOn() function).

Test plan:

0) If required, set OpacHighlightedWords to "Highlight".

1) Search for "history" in the OPAC, and view the details page
   of one of the results. Click on the "Unhighlight" button a
   few times -- after a couple of clicks the button text will
   stop toggling and you won't be able to remove the keyword
   highlight any more.

2) Apply this patch.

3) Repeat step 1): this time you should be able to toggle the
   search keyword highlighting no matter how many times you
   click on the "Unhighlight" / "Highlight" button.

4) Repeat step 1), but this time search for "century history":
   you should be able to toggle search keyword highlighting in
   the details page on and off repeatedly without any problems.
Comment 2 Lucas Gass 2023-06-14 14:47:08 UTC
Created attachment 152338 [details] [review]
Bug 34005: Move button toggling code outside JS for loop

When OpacHighlightedWords is set to "Highlight", if you search
in the OPAC and the number of keywords is odd (1, 3, 5, etc.),
then the "Unhighlight" / "Highlight" button will stop working
after a couple of clicks. The relevant commit is 9cb89b46397
from v20.11.00, where the code responsible for toggling the
"Unhighlight" / "Highlight" button was moved inside the JS
for loop that cycles through each term in the array of search
terms. This explains why the toggling only works when an even
number of search terms is being used.

This patch fixes that, by placing the button toggling code in
its original position (outside the JavaScript for loop of the
highlightOn() function).

Test plan:

0) If required, set OpacHighlightedWords to "Highlight".

1) Search for "history" in the OPAC, and view the details page
   of one of the results. Click on the "Unhighlight" button a
   few times -- after a couple of clicks the button text will
   stop toggling and you won't be able to remove the keyword
   highlight any more.

2) Apply this patch.

3) Repeat step 1): this time you should be able to toggle the
   search keyword highlighting no matter how many times you
   click on the "Unhighlight" / "Highlight" button.

4) Repeat step 1), but this time search for "century history":
   you should be able to toggle search keyword highlighting in
   the details page on and off repeatedly without any problems.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 3 Martin Renvoize 2023-06-26 11:17:09 UTC
Created attachment 152677 [details] [review]
Bug 34005: Move button toggling code outside JS for loop

When OpacHighlightedWords is set to "Highlight", if you search
in the OPAC and the number of keywords is odd (1, 3, 5, etc.),
then the "Unhighlight" / "Highlight" button will stop working
after a couple of clicks. The relevant commit is 9cb89b46397
from v20.11.00, where the code responsible for toggling the
"Unhighlight" / "Highlight" button was moved inside the JS
for loop that cycles through each term in the array of search
terms. This explains why the toggling only works when an even
number of search terms is being used.

This patch fixes that, by placing the button toggling code in
its original position (outside the JavaScript for loop of the
highlightOn() function).

Test plan:

0) If required, set OpacHighlightedWords to "Highlight".

1) Search for "history" in the OPAC, and view the details page
   of one of the results. Click on the "Unhighlight" button a
   few times -- after a couple of clicks the button text will
   stop toggling and you won't be able to remove the keyword
   highlight any more.

2) Apply this patch.

3) Repeat step 1): this time you should be able to toggle the
   search keyword highlighting no matter how many times you
   click on the "Unhighlight" / "Highlight" button.

4) Repeat step 1), but this time search for "century history":
   you should be able to toggle search keyword highlighting in
   the details page on and off repeatedly without any problems.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2023-06-26 11:17:32 UTC
Clear and working fix, Passing QA
Comment 5 Tomás Cohen Arazi 2023-07-07 15:12:09 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 6 Martin Renvoize 2023-07-18 09:24:44 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 7 Matt Blenkinsop 2023-07-18 13:46:31 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x