Allow form redirects to ignore ?destination query parameter

#10 works well. But we can use this in EntityForm while waiting for a real solution accepted

$this->requestStack->getCurrentRequest()->query->remove('destination');

📌 Revisit the redirect to 'add block' form in the 'add block content' form Fixed appears to be blocked by this as well.

Tried starting a 11.x branch but no luck so patch it is

I started from MR 2678

Updated the RedirectTest to check the path not destination (since it was ignored)
Updated typehinting for D10
Updated trigger_error message, still needs a change record

I undid the change for core/modules/image/src/ImageStyleListBuilder.php as I wasn't sure it was needed? If it is then the solution needs to be looked at because currently when adding an image style
1. The url is messed up with the destination parameter now appearing.
2. When saving an image style instead of going back to the individual style you get redirected to the whole styles page.

Had issues with the interdiff so just made a diff.

Or this approach where I add the changes back to ImageStyleListBuilder but in RedirectResponseSubscriber I don't add the destination paramter for item 55.1

Which also solved 55.2

1. +++ b/core/core.services.yml
   --- a/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
   +++ b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
   
   +++ b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
   @@ -59,12 +66,21 @@ public function checkRedirectUrl(ResponseEvent $event) {
   +        else {
   +          $target_url = $response->getTargetUrl();
   +          $query_string = parse_url($target_url, PHP_URL_QUERY) ?: '';
   +          parse_str($query_string, $query);
   +          $target_url = substr($target_url, 0, strpos($target_url, '?')) . '?' . http_build_query($query);
   +          $response->setTargetUrl($target_url);
   

   Why do we need this 🤔

2. +++ b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
   @@ -25,6 +25,13 @@ class RedirectResponseSubscriber implements EventSubscriberInterface {
   +  protected $ignoreDestination = FALSE;
   

   We can use property type declaration 😇

3. +++ b/core/lib/Drupal/Core/Form/FormState.php
   @@ -136,6 +136,13 @@ class FormState implements FormStateInterface {
   +  protected $ignoreDestination = FALSE;
   

   We could use property type declaration here too 😇

4. +++ b/core/lib/Drupal/Core/Form/FormStateDecoratorBase.php
   --- a/core/lib/Drupal/Core/Form/FormStateInterface.php
   +++ b/core/lib/Drupal/Core/Form/FormStateInterface.php
   
   +++ b/core/lib/Drupal/Core/Form/FormStateInterface.php
   @@ -158,6 +158,26 @@ public function setRedirectUrl(Url $url);
   +  public function ignoreDestination(bool $status = TRUE);
   

   Wondering if this should be setIgnoreDestination given that this can be used for disabling this behavior too?

#57.1 = I'm trying to remember but lets see what happens without it.
#57.2 = added type
#57.3 = added type
#57.4 = changed all function calls to setIgnoreDestination

Added back that snippet from 57.1

Seems to grab the URL minus the destination.

Also started the CR https://www.drupal.org/node/3375113 →

Updated issue credits

1. +++ b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
   @@ -59,12 +66,21 @@ public function checkRedirectUrl(ResponseEvent $event) {
   +          $target_url = $response->getTargetUrl();
   +          $query_string = parse_url($target_url, PHP_URL_QUERY) ?: '';
   +          parse_str($query_string, $query);
   +          $target_url = substr($target_url, 0, strpos($target_url, '?')) . '?' . http_build_query($query);
   +          $response->setTargetUrl($target_url);
   

   Can we get a comment explaining what this is doing? It seems to parse the query, and then un-parse it and re-set it on the same url?

2. +++ b/core/lib/Drupal/Core/Form/FormSubmitter.php
   @@ -34,10 +42,17 @@ class FormSubmitter implements FormSubmitterInterface {
   +  public function __construct(RequestStack $request_stack, UrlGeneratorInterface $url_generator, RedirectResponseSubscriber $redirect_response_subscriber = NULL) {
   

   shouldn't this be ?RedirectResponseSubscriber

Will work on this tomorrow. If I don’t get to in 2 days anyone can have at it.

Assigning to myself as a submaintainer

Lets try this.

Address #60

Updated our patch for this issue during a 10.1 upgrade and something has broken with a mix of $form_state->setRedirect() and setIgnoreDestination() - now the form doesn't redirect at all, instead the page refreshes with an empty query string.

Debugging through it, somehow $target_url is becoming literally a ? after this line:

$target_url = substr($target_url, 0, strpos($target_url, '?')) . '?' . http_build_query($query);

Where before the line, target_url is http://127.0.0.1:8080/admin/content/service-status-messages/560/scheduled-transitions

and $query is empty.

I guess we need test coverage for this and only do this if $target_url contains a ?

Weird cause this test case should be testing exactly that

    // Test basic redirection, ignoring the 'destination' query parameter.
    $options['query']['destination'] = $this->randomMachineName();
    $edit = [
      'redirection' => TRUE,
      'destination' => $this->randomMachineName(),
      'ignore_destination' => TRUE,
    ];
    $this->drupalGet($path, $options);
    $this->submitForm($edit, 'Submit');
    $this->assertSession()->addressEquals($path);

Addressing this bug, looks like it was first introduced in #55

The snippet was added in the MR to retain the destination in the url

Fixed, also added coverage for when the redirect url itself contains a query param.

@smustgrave no, the MR still had the correct check in the test https://git.drupalcode.org/project/drupal/-/merge_requests/2678/diffs#62...

The last submitted patch, 69: 2950883-68--failing-test.patch, failed testing. View results →

Was referring to $target_url = substr($target_url, 0, strpos($target_url, '?')) . '?' . http_build_query($query);

But #69 is all green and still working.

Would be nice to get this in as it's a blocker for a block_content redirect.

The last submitted patch, 69: 2950883-68--failing-test.patch, failed testing. View results →

+++ b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
@@ -59,12 +66,25 @@ public function checkRedirectUrl(ResponseEvent $event) {
+          // The 'destination' parameter should remain in the query but be
+          // unused by the target_url.
+          $target_url = $response->getTargetUrl();
+          if (str_contains($target_url, '?')) {
+            $query_string = parse_url($target_url, PHP_URL_QUERY) ?: '';
+            parse_str($query_string, $query);
+            $target_url = substr($target_url, 0, strpos($target_url, '?')) . '?' . http_build_query($query);
+          }
+          $response->setTargetUrl($target_url);

Sorry if I'm being daft here, but I don't understand what this code is doing.

It seems to
a) get the query
b) if it contains a ? parse the query
c) recreate the same query

https://3v4l.org/XYAX5 - the body of queryManip is copy pasted from here

Confirming #57, #60 and #74 - this code is redundant.

What it looks to be trying to achieve is what's described in #11 and #44 which while it might be nice, is IMO out of scope of this issue. If people want this functionality (retaining the destination query param on the redirected URL) then please file a follow up. That way we can discuss specific use cases and have a more targeted discussion.

Awesome!

creating a new IgnoreDestinationRedirectResponse class and modifying the RedirectResponseSubscriber accordingly.
1. Create the IgnoreDestinationRedirectResponse Class:
Create a new class that extends the Symfony\Component\HttpFoundation\RedirectResponse class. This class will allow you to ignore the ?destination parameter in certain cases.

2. Implement the FormStateInterface Helpers:
In Drupal, you can use FormStateInterface to handle form-related functionality. Add the following methods to the FormStateInterface:
<?php
use Drupal\Core\Form\FormStateInterface;

interface FormStateInterface extends \ArrayAccess {

// ...

/**
* Gets the status of whether to ignore the ?destination query parameter in redirects.
*
* @return bool
* TRUE to ignore ?destination, FALSE otherwise.
*/
public function getIgnoreDestination();

/**
* Sets the status of whether to ignore the ?destination query parameter in redirects.
*
* @param bool $status
* TRUE to ignore ?destination, FALSE otherwise.
*
* @return $this
*/
public function ignoreDestination($status);
}

3. Implement RedirectResponseSubscriber:
Modify the RedirectResponseSubscriber to check if the redirect should ignore the ?destination parameter based on the IgnoreDestinationRedirectResponse and FormStateInterface settings.

4. Use the New Helpers in Forms:
Now you can use the new FormStateInterface methods in your forms to control whether the redirect should ignore the ?destination parameter or not. For example:
<?php
use Drupal\Core\Form\FormStateInterface;

public function submitForm(array &$form, FormStateInterface $form_state) {
// Your form submission logic here.

// Use the ignoreDestination() method to set whether to ignore ?destination or not.
$form_state->ignoreDestination(TRUE);

// Redirect to a specific page without considering ?destination.
$form_state->setRedirect('your.route.name');
}

Follow-up issue created: ✨ Pass on desgination redirect when ignoring form redirects' ?destination query parameter Active

1. +++ b/core/lib/Drupal/Core/Form/FormSubmitter.php
   @@ -27,6 +28,13 @@ class FormSubmitter implements FormSubmitterInterface {
   +  protected $redirectResponseSubscriber;
   

   We can typehint this

2. +++ b/core/lib/Drupal/Core/Form/FormSubmitter.php
   @@ -34,10 +42,17 @@ class FormSubmitter implements FormSubmitterInterface {
   +      @trigger_error('Calling ' . __CLASS__ . '::__construct() without the $redirect_response_subscriber argument is deprecated in drupal:10.2.0 and is required in drupal:11.0.0. See https://www.drupal.org/node/XXXX', E_USER_DEPRECATED);
   

   This needs an actual change record URL

CR added https://www.drupal.org/node/3377297 →

Type and comment updated.

Feedback addressed

Committed 92abbb5 and pushed to 11.x. Thanks!

Thanks all, we got there in the end.

Publishing both change records.

Opened a minor follow-up for this: 🐛 \Drupal\Core\Form\FormSubmitter::__construct $redirect_response_subscriber missing default value Fixed .

Automatically closed - issue fixed for 2 weeks with no activity.