Bug 11358 - Use Amazon's HTTPS URLs for covers when using HTTPS in Koha.
Summary: Use Amazon's HTTPS URLs for covers when using HTTPS in Koha.
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords: Academy
Depends on: 10944
Blocks: 17137
  Show dependency treegraph
 
Reported: 2013-12-09 18:36 UTC by Frédérick Capovilla
Modified: 2017-01-10 19:28 UTC (History)
15 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Use HTTPS Amazon images when Koha is using HTTPS. (32.74 KB, patch)
2013-12-09 18:36 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
Bug: 11358 - Use HTTPS Amazon images when Koha is using HTTPS. (15.20 KB, patch)
2015-01-14 22:16 UTC, Justin Vos
Details | Diff | Splinter Review
Bug: 11358 - Use HTTPS Amazon images everywhere (20.88 KB, patch)
2015-11-26 17:32 UTC, Charles Farmer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Frédérick Capovilla 2013-12-09 18:36:53 UTC Comment hidden (obsolete)
Comment 1 Katrin Fischer 2013-12-09 18:56:26 UTC
Hi Frederick, I think this might be a duplicate - bug 10944
Comment 2 Liz Rea 2013-12-10 02:42:16 UTC
it's not entirely duplicate - he covers some things we didn't cover in the other patch - such as shelves and lists and such. Reconcile this one with that and we should have complete coverage of ssl for amazon on the opac.

Liz
Comment 3 Mark Tompsett 2014-04-24 01:59:31 UTC
There is no test plan. A thorough test plan speeds sign offs, Frédérick.
Comment 4 Koha Team University Lyon 3 2014-06-18 14:26:31 UTC
<h1>Something went wrong !</h1>Applying: Use HTTPS Amazon images when Koha is using HTTPS.
Using index info to reconstruct a base tree...
<stdin>:67: trailing whitespace.
        if ((this.src.indexOf('images.amazon.com') >= 0) || (this.src.indexOf('g-images.amazon.com') >=0) 
<stdin>:68: trailing whitespace.
        || (this.src.indexOf('syndetics.com') >=0) || (this.src.indexOf('images-na.ssl-images-amazon.com') >= 0)) { 
<stdin>:227: trailing whitespace.
        if ((this.src.indexOf('images.amazon.com') >= 0) || (this.src.indexOf('g-images.amazon.com') >=0) 
<stdin>:228: trailing whitespace.
        || (this.src.indexOf('syndetics.com') >=0) || (this.src.indexOf('images-na.ssl-images-amazon.com') >= 0)) { 
<stdin>:253: trailing whitespace.
        if ((this.src.indexOf('images.amazon.com') >= 0) || (this.src.indexOf('g-images.amazon.com') >=0) 
warning: squelched 3 whitespace errors
warning: 8 lines add whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-opensearch.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-readingrecord.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/prog/en/modules/opac-readingrecord.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt
 .
Comment 5 Martin Renvoize 2015-01-07 07:52:26 UTC
Did this get folded into bug 10944 in the end?  i.e. is this closable a duplicate fixed?
Comment 6 Mark Tompsett 2015-01-07 16:04:48 UTC
(In reply to Martin Renvoize from comment #5)
> Did this get folded into bug 10944 in the end?  i.e. is this closable a
> duplicate fixed?

Partially. I started looking that the Splinter Review between the two and noticed koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt differs. This one has more! This should be reworked conflict free and a full testing plan given, in my opinion.
Comment 7 Martin Renvoize 2015-01-14 08:16:38 UTC
This might be another good one for the Academy, marking as such..  Hopefully it's not too difficult to pull out the missing bits so the patch applies again... and get to the bottom of what this patch attempts to achieve that differs to the related bug.
Comment 8 Justin Vos 2015-01-14 22:16:43 UTC Comment hidden (obsolete)
Comment 9 Blou 2015-03-16 16:04:14 UTC
I obsolete the old patch, as it is my understanding the new patch REPLACES it.
Anyway, otherwise, it is virtually impossible to apply the old one.

This remains to be tested/signed off.  We'll test it, but as the sponsor, we can't sign it off.
Comment 10 Mirko Tietgen 2015-06-25 14:19:57 UTC
This needs a rebase.

Applying: Bug: 11358 - Use HTTPS Amazon images when Koha is using HTTPS.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc
Auto-merging koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css
Failed to merge in the changes.
Patch failed at 0001 Bug: 11358 - Use HTTPS Amazon images when Koha is using HTTPS.
Comment 11 Mark Tompsett 2015-06-25 21:20:00 UTC
Looking further, the lack of test plan seems to highlight the issues still outstanding.

There are https lack of use in the staff client.
There is at least one https use of amazon images not changed in OPAC.
Even if this was rebased, I think there are problems.
So, whoever fixes this first, please include a test plan.
Some of these changes are pointless in functionality, because you can access https on an http page without triggering security issues but not the other way around. Sometimes it is just better to https all the time.
Comment 12 Charles Farmer 2015-11-26 17:32:44 UTC
Created attachment 45182 [details] [review]
Bug: 11358 - Use HTTPS Amazon images everywhere

Tried to solve the conflicts. Additionnaly, edited files that were still using "http://images.amazon.com". Please test thoroughly.

Using HTTPS on an HTTP page shouldn't trigger security issues. I pruned some templates to use HTTPS whenever we're fetching an image from Amazon instead of switching on the using_https variable.
Comment 13 Mark Tompsett 2015-11-26 19:17:35 UTC
(In reply to Charles Farmer from comment #12)
> Please test thoroughly.

Can't test thoroughly WITHOUT A TEST PLAN.
See comment #3, comment #6, comment #11. I keep asking for one, and keep not getting one. Post a thorough test plan, and I will test.
Comment 14 Charles Farmer 2015-11-26 23:21:46 UTC
Test plan (do all these tests on HTTP)

To test the changes in staff-global.css, doc-head-close.inc and amazonimages.js :

	1) Go to your INTRANET
	2) Under the search bar, click on "Search the catalog"
	3) Enter a criteria for which you'll get more than one result
	4) Documents without an amazon image should have a "No cover image available" placeholder
	5) (hard part) Somehow, disable javascript in your browser
		1) Under firefox, click F12
		2) Click the gear icon at the top right side of the developer interface (4th item from the right)
		3) Under "Advanced settings", you'll find "Disable javascript": Click it.
	6) After a forced page reload, instead of a placeholder image, you should see a 1x1 image.

Intranet:

	1) Go to your INTRANET
	2) Under the search bar, click on "Search the catalog"
	3) Enter a criteria for which you'll get more than one result
	4) Click on an entry with a visible image
	5) The *amazon-bookcoverimg* should come from https://images-na.ssl-images-amazon.com/

OPAC:

	1) Go to your OPAC
	2) Search the library catalog for something
		1) Images in the result page should be HTTPS
	3) Click on anything with an amazon image
	4) The opac-detail page should show an HTTPS image
	5) On the bottom of the page, open the shelf browser
		1) The image should be in HTTPS there too

	===

	1) Go to the page http://<your opac>/cgi-bin/koha/opac-showreviews.pl
	2) Book images should be in HTTPS

//////////

To be quite honest, I don't know where opac-results-grouped.tt is used. I will leave this as an exercise for the reader.
Comment 15 Charles Farmer 2016-02-10 22:06:02 UTC
Anybody?
Comment 16 Jonathan Druart 2016-04-11 11:31:55 UTC
The patch does not apply. Have a look at bug 14571, it may have fixed what you intended to do.
Comment 17 Owen Leonard 2016-05-03 16:38:35 UTC
Fixed by Bug 14571