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.

Files: 
CommentFileSizeAuthor
#5 password_confirm_js_settings-1937686-5.patch680 bytesdboulet
PASSED: [[SimpleTest]]: [MySQL] 53,214 pass(es).
[ View ]
#3 settings_dont_get_added_twice8.patch1.7 KBfranz
PASSED: [[SimpleTest]]: [MySQL] 52,445 pass(es).
[ View ]
#1 settings_dont_get_added_twice.patch1.68 KBfranz
PASSED: [[SimpleTest]]: [MySQL] 39,997 pass(es).
[ View ]

Comments

Assigned:franz» Unassigned
Status:Active» Needs review
StatusFileSize
new1.68 KB
PASSED: [[SimpleTest]]: [MySQL] 39,997 pass(es).
[ View ]

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

Issue tags:+Needs tests

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

Version:7.x-dev» 8.x-dev
StatusFileSize
new1.7 KB
PASSED: [[SimpleTest]]: [MySQL] 52,445 pass(es).
[ View ]

Issue seems present in D8, re-rolled.

It’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_added variable 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 in drupal_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

Title:user_form_process_password_confirm() doesn't properly detect duplicate js settingsJS settings for password_confirm elements are not attached when $form_state['rebuild] = TRUE
StatusFileSize
new680 bytes
PASSED: [[SimpleTest]]: [MySQL] 53,214 pass(es).
[ View ]

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

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