Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EDS Enabled cart doesn't clear sessions when browser is closed #6

Closed
wizzyrea opened this issue May 20, 2015 · 13 comments
Closed

EDS Enabled cart doesn't clear sessions when browser is closed #6

wizzyrea opened this issue May 20, 2015 · 13 comments

Comments

@wizzyrea
Copy link
Contributor

The normal way a cart is used in Koha is as temporary storage - its contents are cleared when the window is closed.

With EDS enabled and the files patched, the sessions are no longer cleared - closing the window and reopening it and then clicking on the "0 items in your cart," there will still be discovery things there.

@minusdavid
Copy link
Contributor

I've noticed this as well. If EDS is enabled, the cart won't be cleared even for regular Koha items.

It looks like the cart info is being stored in bib_list cookies AND in local storage, but I don't think they're consistently updated.

At the moment, my cart says there are 18 items in it when there are actually 2. And even those two were actually from a previously cleared session...

@minusdavid
Copy link
Contributor

After doing an "Empty and clear", I now have "-1" items in my cart... although after I tried to open that -1 item, it reset everything seemingly correctly...

@minusdavid
Copy link
Contributor

Ahhh, I see the problem with the "18" items.

It's because of "jbib_list.length-1" (https://github.com/ebsco/edsapi-koha-plugin/blob/master/Koha/Plugin/EDS/js/EDSScript.js#L102). That's not getting the length of an array. It's getting the length of the string. So jbib_list is "12345678/901234567/" with a length of 19 - 1 = 18.

This goes back to wizzyrea's original problem... the cart shouldn't preserve data from previous sessions.

I assume local storage is needed for using the cart with Discovery, but perhaps another cookie should be used to determine if it's a new session or an on-going session...

@minusdavid
Copy link
Contributor

Going to open some new issues and stop hijacking wizzyrea's...

@minusdavid
Copy link
Contributor

Related to this problem... if you remove all items from the cart using the "Remove" link from within the cart, you'll always have 1 item left in local storage.

The cart will say there are 0 items if you click on it, and the search results will look OK, but if you re-open the cart or close the window and re-open it, the cart will re-gain that 1 item.

@minusdavid
Copy link
Contributor

The more I think about this, the more I don't understand why "bib_list" is stored in local storage... will chat to Alvet about this today...

@alvetm
Copy link
Contributor

alvetm commented Sep 28, 2015

Apparently these 2 lines caused the problem.
//if (basketcount != jbib_list.split('/').length - 1)
//updateBasket(jbib_list.length -1);

This should be fixed in the latest versions of the plugin but will keep this open to allow for testing and confirmation.

@minusdavid
Copy link
Contributor

Sorry, Alvet, but that wasn't the problem in this issue. I hijacked this issue with the count, but the actual problem is that the local storage is preserved between browser sessions. Typically, the Koha cart is emptied when a browser session ends because the cookie is destroyed, and that's the desired behaviour especially in regard to user privacy.

I think it would be wise to consider using sessionStorage instead of localStorage: http://stackoverflow.com/questions/18633715/remove-localstorage-on-closing-browser-javascript

@alvetm
Copy link
Contributor

alvetm commented Sep 28, 2015

You are right; data should be removed on unload. I am using jStorage as it manages cross-browser (no HTML5) and it is set to retain for 30 minutes (by default but configurable) after which it expires but I’ll look at something that clears this data on unload or something similar.

$.jStorage.set('edsKnownItems',JSON.stringify(data),{TTL:edsConfig.cookieexpiry_60_1000})

@minusdavid
Copy link
Contributor

The default isn't 30 minutes. The default is eternity because the default value of cookieexpiry is an alphanumeric string and not a number. The name cookieexpiry seems to be a misnomer as well, since it's really local storage expiry rather than cookie expiry...

I've been looking at the TTL value more...

If the bib_list expires and then you close the browser, the cart will be empty when you re-open it because you won't have anything in local storage.

Otherwise, it looks like EDSScript.js will stuff the contents of localStorage's "bib_list" into the document cookie... whether that's for /cgi-bin/koha or /plugin/Koha/Plugin/EDS/opac depending on what page is visited.

By the way, if you set the expiry value too low, the cart won't work, because the local storage "bib_list" is only being set when in the opac-basket.pl.

The following lines should be used in the search results but they're only called in the opac-basket.pl specific block.
$('.addtocart').click(function () {
$.jStorage.set("bib_list", $.cookie("bib_list"), { TTL: edsConfig.cookieexpiry * 60 * 1000 });
});
$('.cartRemove').click(function () {
$.jStorage.set("bib_list", $.cookie("bib_list"), { TTL: edsConfig.cookieexpiry * 60 * 1000 });
});

@minusdavid
Copy link
Contributor

Actually, sorry, that bit about the cart not working because of the expiry isn't quite right.

What I meant was... the local storage "bib_list" doesn't get set without visiting the basket. So if you add a few items to the cart, and then go to "Home" or any regular Koha page, your items won't appear in the cart because they weren't in local storage to be sucked up and stuffed into the cookie.

They'll only appear elsewhere in Koha if you visit the opac-basket.pl... and then only if the expiry doesn't destroy them first.

@minusdavid
Copy link
Contributor

Personally, I'm starting to wonder if the regular "bib_list" cookie has a path of "/cgi-bin/koha" by intention or by accident. My guess would be that it's unintentional.

The majority of cookies in Koha use the root path, as they're set by CGI.pm which uses a default root path. The cart uses basket.js which uses window.document.cookie which uses the path of the document by default... which is "/cgi-bin/koha". If we change that to use the root path instead... we could spare a lot of grief.

@cambw
Copy link
Collaborator

cambw commented May 17, 2018

This has been fixed in the newer version of the plugin.
I've updated to make sure that cookies set are Path cookies, and stopping the use of jStorage for the cart.

@cambw cambw closed this as completed May 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants