Problem/Motivation
It's possible I am missing a great design decision here -- wouldn't be the first time -- but I do not understand why CONFIRMATION_URL and CONFIRMATION_URL_MESSAGE use $this->setTrustedRedirectUrl($form_state, $redirect_url); while CONFIRMATION_PAGE uses $form_state->setRedirectUrl. Indeed upgrading from WF5 to WF6 gives us an error requiring a TrustedRedirectResponse.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 3217807-12.patch | 657 bytes | jrockowitz |
| #4 | language.negotiation.yml.txt | 418 bytes | ghost of drupal past |
| #2 | 3217807_2.patch | 826 bytes | ghost of drupal past |
Comments
Comment #2
ghost of drupal pastComment #3
jrockowitz commentedI think the assumption is the CONFIRMATION_PAGE URL is always internal.
Can you please provide the steps required or an example webform that can be used to reproduce the error?
Comment #4
ghost of drupal pastSorry. Internal can mean absolute if the language negotiation is per domain.
Comment #6
jrockowitz commented@Charlie ChX Negyesi That makes sense.
Comment #8
jrockowitz commentedComment #10
jrockowitz commentedArg!!!! This change just major regressions in one of my applications because I am altering the using redirect URL
$form_state-getRedirectUrl()and now the trusted redirect is using the$form_state->getResponse(). I am inclined to consider reverting this change.Comment #11
ghost of drupal pastI did (a lot) more digging and the issue we are hitting is #2643466: "Redirects to external URLs are not allowed by default" for domain-based URL language detections.
We are good to revert here.
I just went with the easy thing, you know? if two things are doing what works why not third? but if it breaks stuff, we can fix the underlying issue.
Thanks for all your hard work on this module.
Comment #12
jrockowitz commentedThanks for doing the research into the source of the issue.
Comment #13
ghost of drupal pastOh I thought you'll just do a git revert. But I can RTBC this for sure :)
Comment #16
jrockowitz commented