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:

  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.

#5 password_confirm_js_settings-1937686-5.patch680 bytesdboulet
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch password_confirm_js_settings-1937686-5.patch. Unable to apply patch. See the log in the details link for more information. 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


franz’s picture

Assigned: franz » Unassigned
Status: Active » Needs review
1.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.

franz’s picture

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.

franz’s picture

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

Issue seems present in D8, re-rolled.

dboulet’s picture

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

dboulet’s picture

Title: user_form_process_password_confirm() doesn't properly detect duplicate js settings » JS settings for password_confirm elements are not attached when $form_state['rebuild] = TRUE
680 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch password_confirm_js_settings-1937686-5.patch. Unable to apply patch. See the log in the details link for more information. 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.

franz’s picture

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.

Status: Needs review » Needs work

The last submitted patch, 5: password_confirm_js_settings-1937686-5.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.