$form_state['redirect'] is clobbered if set in a form builder function.
if this is by design, then we can convert this to a docs bug, because all core devs i asked in IRC (including davereid and catch) thought it should work if set in a form builder function.
attached is a test-only patch to illustrate the problem.
the code that does the dirty work starts at line 334 in form.inc:
if ($check_cache) {
$uncacheable_keys = array_flip(array_diff(form_state_keys_no_cache(), array('always_process', 'temporary')));
$form_state = array_diff_key($form_state, $uncacheable_keys);
$form_state += $form_state_before_retrieval;
}
}
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | drupal-1251616-8.patch | 2.64 KB | dawehner |
| #4 | drupal8.form-redirect.4.patch | 2.6 KB | sun |
| #1 | form.redirect.patch | 2.67 KB | Anonymous (not verified) |
Comments
Comment #1
Anonymous (not verified) commentedComment #3
chx commentedi remember this one as well from long, long time ago. you sure this hasnt been reported before?
Comment #4
sunThis has been questioned and requested a couple of times in the past already, so it's definitely time to make it happen. I wasn't able to find a dedicated/focused, existing issue for this though.
Attached patch extracts the required changes from #1789768: search_box_form_submit() improperly calls form_set_error(). It doesn't contain the test additions from #1 yet.
Please note that #736564: Add #redirect property for form #type 'submit' to eliminate various submit handlers essentially wants to complement this with a button-level definition of #redirect. That and this issue together will make sense, as they're both moving the redirection target into the constructor/declaration layer.
Comment #5
sunNote that the test changes are required, since language_negotiation_configure_url_form() defines a redirect target in the form constructor already (instead of the form submission handler), and apparently, someone has authored the test coverage based on the actual behavior instead of the code's intended behavior.
I manually tested the language negotiation form user interface to verify that the original intention of the code is correct. Thus, this patch fixes an additional UX bug in the language negotiation admin interface, along with bogus test expectations.
Comment #6
sun#4: drupal8.form-redirect.4.patch queued for re-testing.
Comment #8
dawehnerJust a rerole.
Comment #9.0
(not verified) commentedmake summary not fail to illustrate what i'm saying
Comment #21
quietone commentedDiscussed at BugSmash group triage. larowlan pointed out that this is outdated due to having
$form_state->setRedirectnow.Therefore, closing as outdated. If this is incorrect reopen the issue, by setting the status to 'Active', and add a comment explaining what still needs to be done.
Thanks!
Comment #22
quietone commentedSetting the correct status.