PKP Bugzilla – Bug 6670
Redirects are broken by Firefox's redirect blocking
Last modified: 2013-01-28 11:50:03 PST
When "Warn me when web sites try to redirect or reload the page" is enabled in Firefox's options (Options > Advanced > General), it breaks the redirects used in OCS 22.214.171.124. This happens with Firefox 4 and 5beta.
Most other sites work fine, with a "this site tried to redirect [allow]" notification, but whatever mechanism OCS uses for its redirects are not detected by Firefox but are blocked, and the user ends up at a blank page.
Reloading on the blank page takes the user to the desired location, but if it was a form submission, the submission is made twice.
This may well be a result of bug #3520 -- you could try reversing that commit if you like, but we'd need to find a solution that works in both cases.
(Filing against 2.3.4 for next release.)
Since Alec has requested feedback...
I think this issue is still occurring with my installation of OJS and Firefox.
I've installed OJS recently purely for testing purposes. In Firefox I often get blank screens, e.g. upon login/logout. After a refresh I get the standard 'this site is trying to redirect you' warning. If I click Allow all goes well. In Internet Explorer all this doesn't happen - everything works perfectly. Given Allingham descriptions I am confident I am experiencing the same issue, even though he was referring to OCS at the time.
OJS version 126.96.36.199 installed on Linux with Apache 2.0.46, PHP 5.3.3 and MySQL 4.1.21. Using Firefox 11 and Internet Explorer 8 on Windows 7 in the test.
Thanks; the code is indeed shared between OJS and OCS so it's likely you're seeing the same issue. Scheduling against 2.3.8.
Reverting the commit for bug #3520 does indeed fix the issue.
One solution without reverting that would be to do something like
echo "Redirect to <a href='$url'>$url</a>";
before the header() call in PKPRequest.inc.php's redirectURL method. That way the redirect still works if the browser isn't stopping them such as in this case, and if the browser is stopping them the user still sees a link to the destination page and can click it.
It could be a plain link or a fully styled page, but note that the server has transmitted and the user has received the full page even if the redirect is going to work anyway, which is a little wasteful.
A plain link exists in my commit at https://github.com/tremby/pkp-lib/commit/8d4d2a0f53908168477a3c8cb61f09773795fc92
Bart, have you tried that proposal? OTOH it will cause a "Cannot modify header information - headers already sent by ..." warning and break the redirect header.
Excellent point -- I should get that error but I have tried it and no error. I honestly don't know why that is, and will investigate. I hadn't seen the "Refresh" header before, maybe it works differently somehow.
But to be safe there's no reason the echo can't go /after/ the header, and everything should work. I'll test this when I'm back home and let you know.
Bart, you won't see the error if you have output buffering turned on in your PHP configuration. That's a mixed bag option since some enable it for performance reasons, but I believe that it comes disabled by default.
That makes sense, thanks.
A patch putting the link after the header: https://github.com/tremby/pkp-lib/commit/c34182bee73ed7ec88833bbe1105b5c57b0406e6
Do you want it to send a full page of HTML to frame this link? Or I can do a pull request of this commit.
Deferring to 2.4.1. Bart, I left a comment on that commit re: localizing text.
Thanks guys for looking into this. I'm guessing you'll be adding the final patch to http://pkp.sfu.ca/wiki/index.php/OJS_2.3.7_Recommended_Patches once you're happy with it? Since the issue itself is deferred to v2.4.1?
We tend to keep the "recommended patches" list to high-priority issues, and we haven't had very many reports of this one causing problems. We don't yet have a suitable implementation to include with 2.4.0, so it'll most likely come out with 2.4.1. I'd suggest staying CC'd on this entry if you'd like to apply a patch before we release an official version with the fix included.
Here's a new attempt, using the error template and a new language key:
According to http://pkp.sfu.ca/support/forum/viewtopic.php?f=8&t=9356 there might be side-effects to using Refresh: for bots and crawlers. I wonder if the original modification wasn't mis-diagnosed -- I don't see references to IE ignoring hashes in some quick googling. Suggest starting with some IE testing with the original Location: header.
Bruno, since you've got the best browser testing setup, could you take a look at this one? Start from comment #16; I'm not happy that we're currently straying so far from standard behavior and I'll bet it's unnecessary.
*** Bug 8070 has been marked as a duplicate of this bug. ***
(In reply to comment #17)
> Bruno, since you've got the best browser testing setup, could you take a
> look at this one? Start from comment #16; I'm not happy that we're currently
> straying so far from standard behavior and I'll bet it's unnecessary.
Tested in IE7 and IE8 and both header("Location: $url") and header("Refresh: 0; url=$url") works well with anchors. I was redirected and the anchor was followed.
IE6 is still a concern? I can test if it is.
No, I think at this point we can drop IE6 support. Go ahead and revert bug #3520.
Thanks for the confirmation, Luke. Addressing IE's High security level is outside the scope of this bug, but if you do have any suggestions, feel free to drop them in.
Revert bug 3520
Revert bug 3520