Bug 7657 - Google and OpenLibrary image conflict
Summary: Google and OpenLibrary image conflict
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Owen Leonard
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-06 20:53 UTC by Shane Sammons
Modified: 2014-05-26 21:04 UTC (History)
5 users (show)

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


Attachments
Bug 7657 - Google and OpenLibrary image conflict (3.12 KB, patch)
2013-01-03 18:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Example of book covers after patch is applied. (68.28 KB, image/jpeg)
2013-01-04 17:53 UTC, Kyle M Hall
Details
Bug 7657 - Google and OpenLibrary image conflict (3.13 KB, patch)
2013-01-04 17:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7657 - Google and OpenLibrary image conflict (3.19 KB, patch)
2013-01-15 09:08 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Shane Sammons 2012-03-06 20:53:08 UTC
If you enable Google and Open Library conver images in OPAC you will get the images 2 times each. They seems to generate from similar code and have a conflict.
Comment 1 Magnus Enger 2012-03-07 09:12:46 UTC
I think this would be solved by a solution to Bug 7187 - "Prioritize cover image sources such as Amazon, Google and add fail over i.e. if first source fails, go to next"? 

Marking as duplicate of that bug - please undo if I'm missing something.

*** This bug has been marked as a duplicate of bug 7187 ***
Comment 2 Frédéric Demians 2012-03-07 12:26:41 UTC
Both bug haven't the same scope level. This one would just be a quick fix. Owen already has a jQuery solution he had experimented with on IRC, that could be integrated without that much QA, contrary to the other bug.
Comment 3 Owen Leonard 2012-03-07 13:11:40 UTC
(In reply to comment #2)
> Both bug haven't the same scope level.

Specifically, because the goal of the bug reporter is to be able to enable both Google Books and Open Library simultaneously. That such a configuration is not exactly supported might mean this is better considered an enhancement.
Comment 4 Owen Leonard 2013-01-03 18:00:47 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2013-01-04 17:53:04 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2013-01-04 17:53:29 UTC
This doesn't appear to be working for me. I've attached a screenshot.
Comment 7 Kyle M Hall 2013-01-04 17:54:25 UTC
Cancel that. I misunderstood the issue.
Comment 8 Kyle M Hall 2013-01-04 17:55:14 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2013-01-14 15:19:54 UTC
Owen,

It does not work here.
I enabled the 2 sysprefs: OPACAmazonCoverImages and OpenLibraryCovers.

2 images are displayed. If I disable 1 syspref, I just get 1 image (tested one by one).

The generated html code is:

<a class="p1" href="/cgi-bin/koha/opac-detail.pl?biblionumber=32">
<img class="thumbnail" alt="" src="http://images.amazon.com/images/P/2857045778.01.TZZZZZZZ.jpg">
<span id="openlibrary-thumbnail3" class="2857045778" title="32" style="block">
<img src="http://covers.openlibrary.org/b/id/2177282-S.jpg">
</span>
</a>
Comment 10 Chris Cormack 2013-01-14 19:03:34 UTC
Hmm it is Google and Openlibrary .. not amazon Jonathan. Could that be why it didnt work. 
IE you need to set the google and openlibrary sysprefs not the amazon one.
Comment 11 Jonathan Druart 2013-01-15 08:46:06 UTC
(In reply to comment #10)
> Hmm it is Google and Openlibrary .. not amazon Jonathan. Could that be why
> it didnt work. 
> IE you need to set the google and openlibrary sysprefs not the amazon one.

Oops yes, sorry :)
Too many syspref for covers, I was a little bit lost.

However, it still does not work.

Before the path, I got 4 images:

<td>
<a class="p1" href="/cgi-bin/koha/opac-detail.pl?biblionumber=608">
<span id="gbs-thumbnail19" class="2253115525" title="608" style="block">
<img src="http://bks5.books.google.com/books?id=Dq9LPgAACAAJ&printsec=frontcover&img=1&zoom=5">
<img src="http://covers.openlibrary.org/b/id/6675897-S.jpg">
</span>
<span id="openlibrary-thumbnail19" class="2253115525" title="608" style="block">
<img src="http://bks5.books.google.com/books?id=Dq9LPgAACAAJ&printsec=frontcover&img=1&zoom=5">
<img src="http://covers.openlibrary.org/b/id/6675897-S.jpg">
</span>
</a>
</td>

Now I get

<td>
<a class="p1" href="/cgi-bin/koha/opac-detail.pl?biblionumber=608">
<span id="gbs-thumbnail19" class="2253115525" title="608" style="block">
<img src="http://bks5.books.google.com/books?id=Dq9LPgAACAAJ&printsec=frontcover&img=1&zoom=5">
</span>
<span id="openlibrary-thumbnail19" class="2253115525" title="608" style="block">
<img src="http://covers.openlibrary.org/b/id/6675897-S.jpg">
</span>
</a>
</td>

What I missed?

OpenLibraryCovers ON
GoogleJackets ON
OPACAmazonCoverImages OFF
OPACLocalCoverImages OFF
Comment 12 Chris Cormack 2013-01-15 08:50:40 UTC
That's right, before the patch you should get 4 .. 2 of each showing .. after you should only get 2, 1 of each.

This patch isn't to do anything like only show google or openlibrary .. but to fix the error that if you have them both on, you get 2 google, and 2 open library images.

It isn't doing bug 7187 (priotising image sources), but is a javascript fix.

From Owen

"Specifically, because the goal of the bug reporter is to be able to enable both Google Books and Open Library simultaneously. That such a configuration is not exactly supported might mean this is better considered an enhancement."

So you should get 2 images, one from each .. (if they both have them, if one has one and the other doesnt you should get 1 image and an image not found).

Instead of 4 images, 2 each 

I hope that helps makes more sense
Comment 13 Jonathan Druart 2013-01-15 09:08:03 UTC
Yep, thanks Chris.

So the patch works as expected.

Marked as Passed QA.
Comment 14 Jonathan Druart 2013-01-15 09:08:17 UTC
Created attachment 14591 [details] [review]
Bug 7657 - Google and OpenLibrary image conflict

The JavaScript files which output Google and OpenLibrary cover images
each assumes it is the only source for cover images running. Each script
isn't specific enough to filter out markup generated by the other. This
patch corrects the specificity of the selector looking for markup
related to Google and OpenLibrary covers.

To test, enable both Google and OpenLibrary cover images in the OPAC.
Perform a search which will return results which include titles which
have covers from both services. Confirm that these covers appear
correctly and that "no cover" information still works correctly.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 15 Jared Camins-Esakov 2013-01-16 02:51:39 UTC
This patch has been pushed to master.
Comment 16 Chris Cormack 2013-01-16 06:41:13 UTC
Pushed to 3.10.x will be in 3.10.2