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

Comments

franz’s picture

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

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

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

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

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

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