In the usual case when you submit a form, you get redirected back to the same path, unless there was a destination value in the query portion of the URL. This is not the case for the front page. When you submit a form on the front page you get redirected to variable_get('site_frontpage', 'node'). This is most common with a login form on the front page where after being logged in the user is now at /node rather than just /.

The reason for this is that we directly access $_GET['q'] in several places rather than use the new request_path().

CommentFileSizeAuthor
#18 949230-18-front-page-forms-tests-plus-fix.diff5.25 KBksenzee
#14 949230-14-front-page-forms-tests-plus-fix.diff5.75 KBdalin
#7 949230-7-front-page-forms-tests.diff4.07 KBdalin
#7 949230-7-front-page-forms-tests-plus-fix.diff5.43 KBdalin
#3 949230.diff577 bytesdalin
#1 949230.diff22.25 KBdalin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dalin’s picture

dalin
he/him
Primary language English
Location 🇲🇽, 🇨🇦, 🌍
CreditAttribution: dalin commented
Status: Active » Needs review
FileSize
949230.diff22.25 KB

Here's a first shot at a patch. Lets see what testbot says.

Status: Needs review » Needs work

The last submitted patch, 949230.diff, failed testing.

dalin’s picture

dalin
he/him
Primary language English
Location 🇲🇽, 🇨🇦, 🌍
CreditAttribution: dalin commented
Title: Most instances of $_GET['q'] should be replaced with request_path(). » drupal_redirect_form() should use request_path() instead of $_GET['q'].
Status: Needs work » Needs review
FileSize
949230.diff577 bytes

So 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.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

looks reasonable.

tom_o_t’s picture

tom_o_t CreditAttribution: tom_o_t commented

#3: 949230.diff queued for re-testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Needs a test. I can't quite parse out what the bug report is here.

dalin’s picture

dalin
he/him
Primary language English
Location 🇲🇽, 🇨🇦, 🌍
CreditAttribution: dalin commented
Title: drupal_redirect_form() should use request_path() instead of $_GET['q']. » Forms in blocks on the front page that have no explicit destination should end up at / and not /node
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
949230-7-front-page-forms-tests-plus-fix.diff5.43 KB
949230-7-front-page-forms-tests.diff4.07 KB

I 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()).

Status: Needs review » Needs work

The last submitted patch, 949230-7-front-page-forms-tests-plus-fix.diff, failed testing.

joachim’s picture

joachim CreditAttribution: 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.

dalin’s picture

dalin
he/him
Primary language English
Location 🇲🇽, 🇨🇦, 🌍
CreditAttribution: dalin commented

The 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.

dalin’s picture

dalin
he/him
Primary language English
Location 🇲🇽, 🇨🇦, 🌍
CreditAttribution: dalin commented
Status: Needs work » Needs review

#7: 949230-7-front-page-forms-tests-plus-fix.diff queued for re-testing.

attiks’s picture

attiks CreditAttribution: attiks commented

Patch '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

Status: Needs review » Needs work

The last submitted patch, 949230-7-front-page-forms-tests-plus-fix.diff, failed testing.

dalin’s picture

dalin
he/him
Primary language English
Location 🇲🇽, 🇨🇦, 🌍
CreditAttribution: dalin commented
FileSize
949230-14-front-page-forms-tests-plus-fix.diff5.75 KB

Ok, trying a different approach here that worked for me in a different issue - reset $_GET on setUp() and restore it on tearDown().

dalin’s picture

dalin
he/him
Primary language English
Location 🇲🇽, 🇨🇦, 🌍
CreditAttribution: dalin commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 949230-14-front-page-forms-tests-plus-fix.diff, failed testing.

dalin’s picture

dalin
he/him
Primary language English
Location 🇲🇽, 🇨🇦, 🌍
CreditAttribution: dalin commented

Ok, 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.

ksenzee’s picture

ksenzee
she/her
Location Seattle area
CreditAttribution: ksenzee commented
Version: 7.x-dev » 8.x-dev
FileSize
949230-18-front-page-forms-tests-plus-fix.diff5.25 KB

Reroll courtesy of bug bingo. Let's see what testbot thinks.

ksenzee’s picture

ksenzee
she/her
Location Seattle area
CreditAttribution: ksenzee commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 949230-18-front-page-forms-tests-plus-fix.diff, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

pameeela’s picture

pameeela CreditAttribution: pameeela commented
Version: 8.9.x-dev » 9.3.x-dev
Issue summary: View changes
Issue tags: +Bug Smash Initiative

This still occurs in D9.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.