Bug 6970 - logout not redirecting to login page
Summary: logout not redirecting to login page
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: PATCH-Sent (DO NOT USE) major (vote)
Assignee: Owen Leonard
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-04 18:32 UTC by Nicole C. Engard
Modified: 2012-10-25 23:09 UTC (History)
5 users (show)

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


Attachments
Proposed fix (3.18 KB, patch)
2011-10-05 13:56 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6970 - logout not redirecting to login page (3.21 KB, patch)
2011-10-07 12:43 UTC, Nicole C. Engard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2011-10-04 18:32:17 UTC
Staff logout does not reload the login page. Seems to be an issue with Firefox 7.x since it works properly in 6.x and prior.
Comment 1 Owen Leonard 2011-10-04 18:46:07 UTC
Not only that, it doesn't even log you out.
Comment 2 Ian Walls 2011-10-04 19:53:28 UTC
Problem is on koha-tmpl/intranet-tmpl/prog/en/jd/basket.js, lines 308 and 310 (denoted below by being commented out).

function delBasket(rep) {
    if (rep == undefined){
        rep = confirm(MSG_CONFIRM_DEL_BASKET);
    }
    if (rep) {
        delCookie(nameCookie);
//        document.location = "about:blank";
        updateBasket(0,top.opener);
//        window.close();
    }
}

Removing those lines as above fixes the issue in the latest Firefox; not sure about other browsers, if they'd be required.
Comment 3 Owen Leonard 2011-10-05 13:56:43 UTC Comment hidden (obsolete)
Comment 4 Ian Walls 2011-10-05 14:12:29 UTC
Proposed patch works on FF7 and Chromium 12; need to check other browsers before I can definitely sign off and pass QA.
Comment 5 Nicole C. Engard 2011-10-07 12:42:51 UTC
Works in Safari, FF7 and Chrome on Mac.  Giving my sign off.
Comment 6 Nicole C. Engard 2011-10-07 12:43:24 UTC
Created attachment 5769 [details] [review]
[SIGNED-OFF] Fix for Bug 6970 - logout not redirecting to login page

For some reason the latest version of Firefox doesn't like
the javascript which empties the cart on logout. As Ian points
out, there is code which tries to redirect to "about:blank"
and close the window (since the main context of the function
is the Cart's popup window.

This patch adds a parameter to the delBasket function so
that we can pass a "popup" context to the function and
choose whether or not we want to ask JS to close the window.

Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Comment 7 Nicole C. Engard 2011-10-07 12:44:08 UTC
I think we need a sign off from a windows person and a linux person to please Ian :) anyone else want to test and sign off?
Comment 8 Julian Maurice 2011-10-07 13:47:44 UTC
Works with Firefox 7 and Chromium 14 on Linux.
Signed off.
Comment 9 Magnus Enger 2011-10-07 13:52:08 UTC
I have tested on Ubuntu, with the following results: 

- Chromium: not affected by the problem, works after the patch
- FF7: The patch solves the problem
- Opera 11.10: The patch does not solve the problem (the tab Koha is in is
closed when I click Log out, and when I return to Koha I'm still logged in)

Not sure if we should let the Opera fail stand in the way of a fix for FF? The
latter is probably used far more than the former...
Comment 10 Ian Walls 2011-10-07 14:00:33 UTC
Owen has reported this works in IE8.
Comment 11 Ian Walls 2011-10-07 14:08:15 UTC
Okay, so looks like Linux and Mac are well covered.  We've got positive reports of this working on IE8 on Windows.

I'd like to see the following confirmations:

FF3-7 on Windows
Safari on Windows
IE6-7 on Windows
FF3-6 on Mac

I say FF3 because I'm pretty sure I still have a laptop that runs that, and it's not that old.  But, if we can make a good case for a higher minimum number, then I'm okay with that.  At the very least, FF6 needs to be tested.
Comment 12 Paul Poulain 2011-10-07 15:51:28 UTC
Just a point : I agree we must check FF under windows. But I like not to maximize the number of platforms tested: that's a real problem under FF7, so I think we should not delay too much this patch.
We can ignore sub-versions : 3.6 and 7 is enough : there's an automatic update from 5 to 7 for example, so the possibility to have a library using FF5+ the patch being a problem for FF5 is from far less important compared to ppl using FF7 and suffering the problem !
Comment 13 Ian Walls 2011-10-13 19:34:06 UTC
I've confirmed this works fine for FF4 and FF7 on Windows.

I think this is a big enough problem, and we've tested enough of the cases now, that if a library runs into a specific OS/browser combination that doesn't work, we can offer up interim alternatives until a further patch can be developed.

Marking this with my signoff, and passing QA.
Comment 14 Chris Cormack 2011-10-14 03:00:55 UTC
Pushed, please test
Comment 15 Chris Nighswonger 2011-10-26 18:02:09 UTC
The fix for this bug was published in the 3.4.5 release. If you were the reporter of this bug, please take time to verify the fix and update the status of this bug report accordingly. If the bug is fixed to your satisfaction, please close this report.