Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I have a custom form that uses password_confirm element. During validation step, $form_state['rebuild'] is set, and so, the form is re-processed. When this element is re-processed, however, the settings are not added because of this part:
<?php
static $already_added = FALSE;
if (!$already_added) {
$already_added = TRUE;
$element['#attached']['js'][] = array('data' => $js_settings, 'type' => 'setting');
}
?>As this funcion is being called a second time in the same request, the settings don't get added, which causes a JS error. I'll work on a patch to fix this.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | password_confirm_js_settings-1937686-5.patch | 680 bytes | dboulet |
| #3 | settings_dont_get_added_twice8.patch | 1.7 KB | franz |
| #1 | settings_dont_get_added_twice.patch | 1.68 KB | franz |
Comments
Comment #1
franzThis patch tries to add a kind of feature to drupal_add_js(), although I'm unsure if it's the best way to address this. I also don't know if this kind of thing is needed elsewhere in core.
Comment #2
franzNeed to add tests for:
a - 2 password_confirm form elements on the same page (assert settings are ok)
b - a case similar to the bug description itself.
Comment #3
franzIssue seems present in D8, re-rolled.
Comment #4
dboulet CreditAttribution: dboulet commentedIt’s not clear to me why this function tries to ensure that the settings only get added once—is it for cases where there is more than one password_confirm element on the page?
Can’t we simply get rid of the
$already_addedvariable and allow the settings to be added multiple times? As far as I understand the settings won’t be written to the page multiple times in this case—the multiple instances of the 'password' setting will be merged into one indrupal_get_js().Edit: Here is the issue where the code in question was added: #788166: (Tests needed) password_confirm element breaks if there is more than 1 on a page
Comment #5
dboulet CreditAttribution: dboulet commentedI did a quick test in D7 and it doesn’t look like there is any harm in having the settings added more than once. Needs testing in D8.
Comment #6
franzDo we have a test for that case? This might have changed, but I believe the settings were added with array_merge_recursive() which would cause them to be messed if multiple were added. In any case, we should add both test cases if not already present.
Comment #20
quietone CreditAttribution: quietone at PreviousNext commented@franz, Thank you for reporting this problem and making a patch. We rely on issue reports like this one to resolve bugs and improve Drupal core.
There has been no activity here for 9 years, perhaps this is no longer a problem? The code changed in the patch was removed in #2382557: Change JS settings into a separate asset type.
Is this still a problem?
If you are experiencing this problem on a supported version of Drupal reopen the issue, by setting the status to 'Active', and provide complete steps to reproduce the issue (starting from "Install Drupal core").
Thanks!
Comment #21
quietone CreditAttribution: quietone at PreviousNext commentedThere have been no further reports of this problem, therefor I am closing this issue.
Cheers