Needs work
Project:
Drupal core
Version:
main
Component:
base system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
22 Oct 2010 at 04:21 UTC
Updated:
6 Jul 2021 at 12:10 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
dalinHere's a first shot at a patch. Lets see what testbot says.
Comment #3
dalinSo I think we need to scale this back. Perhaps not all instances need to be replaced. I suppose there would be a performance cost to doing so as well. Other instances of $_GET['q'] could be handled separately if a problem is indeed found.
The attached patch only changes drupal_redirect_form() which fixes the original problem of front-page forms redirecting to a different path.
Comment #4
moshe weitzman commentedlooks reasonable.
Comment #5
tom_o_t commented#3: 949230.diff queued for re-testing.
Comment #6
webchickNeeds a test. I can't quite parse out what the bug report is here.
Comment #7
dalinI started writing tests and got down a bit of a rabit hole. First I realized that we should have tests with ?destination=foo before we tested edge cases where there is no explicit destination, only implied.
The first patch below tests various combinations of (non)front URLs with various combinations of ?destination=foo. It should fail in only one case: a form in a block on the front page will end up at /node rather than / .
The second patch includes the tests plus the fix. But I then discovered that due to changes in the login form since my patch in #3 required me to take a different approach (changes to drupal_get_destination()).
Comment #9
joachim commented@webchick: As I understand it, the OP's report is that this happens:
1. user goes to the front page of the site at /
2. user submits a form on that page
3. page reloads.
4. user (who is the sort of user who actually knows what a location bar is) notices that the site is now at /node. (Or whatever the home page of the site is set to -- the point being it's explicitly a path rather than just /.)
I've noticed this myself... but then again I notice these things.
Comment #10
dalinThe more I work with testbot the less I understand it. All of these tests pass on my local. And I've been staring at the code for ten minutes trying to figure out how testbot could possibly get those results. The only thing that I can think of is that testbot's PHP is somehow preventing the test from setting $_GET. Queueing for retesting.
Comment #11
dalin#7: 949230-7-front-page-forms-tests-plus-fix.diff queued for re-testing.
Comment #12
attiks commentedPatch '949230-7-front-page-forms-tests-plus-fix.diff' tested and works like a charm, redirect goes to / instead of /node
I tested the scenario in #9 with and without the patch
Comment #14
dalinOk, trying a different approach here that worked for me in a different issue - reset $_GET on setUp() and restore it on tearDown().
Comment #15
dalinComment #17
dalinOk, I have no idea why those tests are failing. They work fine on my local machine. AFAICT there's no possible way that testbot could be possibly giving that error message.
Comment #18
ksenzeeReroll courtesy of bug bingo. Let's see what testbot thinks.
Comment #19
ksenzeeComment #29
pameeela commentedThis still occurs in D9.