- Patch #384992 by effulgentsia, sun: drupal_html_id() does not work correctly in AJAX context with multiple forms on page.

merge-requests/26/head
Dries Buytaert 2010-04-07 17:30:43 +00:00
parent 27aa5b1b17
commit d5ce7f5281
10 changed files with 324 additions and 46 deletions

View File

@ -3317,13 +3317,73 @@ function drupal_html_class($class) {
/**
* Prepare a string for use as a valid HTML ID and guarantee uniqueness.
*
* This function ensures that each passed HTML ID value only exists once on the
* page. By tracking the already returned ids, this function enables forms,
* blocks, and other content to be output multiple times on the same page,
* without breaking (X)HTML validation.
*
* For already existing ids, a counter is appended to the id string. Therefore,
* JavaScript and CSS code should not rely on any value that was generated by
* this function and instead should rely on manually added CSS classes or
* similarly reliable constructs.
*
* Two consecutive hyphens separate the counter from the original id. To manage
* uniqueness across multiple AJAX requests on the same page, AJAX requests
* POST an array of all IDs currently present on the page, which are used to
* prime this function's cache upon first invocation.
*
* To allow reverse-parsing of ids submitted via AJAX, any multiple consecutive
* hyphens in the originally passed $id are replaced with a single hyphen.
*
* @param $id
* The ID to clean.
*
* @return
* The cleaned ID.
*/
function drupal_html_id($id) {
$seen_ids = &drupal_static(__FUNCTION__, array());
// If this is an AJAX request, then content returned by this page request will
// be merged with content already on the base page. The HTML ids must be
// unique for the fully merged content. Therefore, initialize $seen_ids to
// take into account ids that are already in use on the base page.
$seen_ids_init = &drupal_static(__FUNCTION__ . ':init');
if (!isset($seen_ids_init)) {
// Ideally, Drupal would provide an API to persist state information about
// prior page requests in the database, and we'd be able to add this
// function's $seen_ids static variable to that state information in order
// to have it properly initialized for this page request. However, no such
// page state API exists, so instead, ajax.js adds all of the in-use HTML
// ids to the POST data of AJAX submissions. Direct use of $_POST is
// normally not recommended as it could open up security risks, but because
// the raw POST data is cast to a number before being returned by this
// function, this usage is safe.
if (empty($_POST['ajax_html_ids'])) {
$seen_ids_init = array();
}
else {
// This function ensures uniqueness by appending a counter to the base id
// requested by the calling function after the first occurrence of that
// requested id. $_POST['ajax_html_ids'] contains the ids as they were
// returned by this function, potentially with the appended counter, so
// we parse that to reconstruct the $seen_ids array.
foreach ($_POST['ajax_html_ids'] as $seen_id) {
// We rely on '--' being used solely for separating a base id from the
// counter, which this function ensures when returning an id.
$parts = explode('--', $seen_id, 2);
if (!empty($parts[1]) && is_numeric($parts[1])) {
list($seen_id, $i) = $parts;
}
else {
$i = 1;
}
if (!isset($seen_ids_init[$seen_id]) || ($i > $seen_ids_init[$seen_id])) {
$seen_ids_init[$seen_id] = $i;
}
}
}
}
$seen_ids = &drupal_static(__FUNCTION__, $seen_ids_init);
$id = strtr(drupal_strtolower($id), array(' ' => '-', '_' => '-', '[' => '-', ']' => ''));
// As defined in http://www.w3.org/TR/html4/types.html#type-name, HTML IDs can
@ -3334,14 +3394,15 @@ function drupal_html_id($id) {
// characters as well.
$id = preg_replace('/[^A-Za-z0-9\-_]/', '', $id);
// Ensure IDs are unique. The first occurrence is held but left alone.
// Subsequent occurrences get a number appended to them. This incrementing
// will almost certainly break code that relies on explicit HTML IDs in forms
// that appear more than once on the page, but the alternative is outputting
// duplicate IDs, which would break JS code and XHTML validity anyways. For
// now, it's an acceptable stopgap solution.
// Removing multiple consecutive hyphens.
$id = preg_replace('/\-+/', '-', $id);
// Ensure IDs are unique by appending a counter after the first occurrence.
// The counter needs to be appended with a delimiter that does not exist in
// the base ID. Requiring a unique delimiter helps ensure that we really do
// return unique IDs and also helps us re-create the $seen_ids array during
// AJAX requests.
if (isset($seen_ids[$id])) {
$id = $id . '-' . ++$seen_ids[$id];
$id = $id . '--' . ++$seen_ids[$id];
}
else {
$seen_ids[$id] = 1;

View File

@ -200,6 +200,12 @@ Drupal.ajax.prototype.beforeSubmit = function (form_values, element, options) {
// Disable the element that received the change.
$(this.element).addClass('progress-disabled').attr('disabled', true);
// Prevent duplicate HTML ids in the returned markup.
// @see drupal_html_id()
$('[id]').each(function () {
form_values.push({ name: 'ajax_html_ids[]', value: this.id });
});
// Insert progressbar or throbber.
if (this.progress.type == 'bar') {
var progressBar = new Drupal.progressBar('ajax-progress-' + this.element.id, eval(this.progress.update_callback), this.progress.method, eval(this.progress.error_callback));

View File

@ -153,7 +153,7 @@ function field_multiple_value_form($field, $instance, $langcode, $items, &$form,
$title = check_plain(t($instance['label']));
$description = field_filter_xss(t($instance['description']));
$wrapper_id = drupal_html_class($field_name) . '-add-more-wrapper';
$wrapper_id = drupal_html_id($field_name . '-add-more-wrapper');
$field_elements = array();
$function = $instance['widget']['module'] . '_field_widget_form';
@ -241,7 +241,7 @@ function theme_field_multiple_value_form($variables) {
$output = '';
if ($element['#cardinality'] > 1 || $element['#cardinality'] == FIELD_CARDINALITY_UNLIMITED) {
$table_id = $element['#field_name'] . '_values';
$table_id = drupal_html_id($element['#field_name'] . '_values');
$order_class = $element['#field_name'] . '-delta-order';
$required = !empty($element['#required']) ? '<span class="form-required" title="' . t('This field is required. ') . '">*</span>' : '';

View File

@ -135,7 +135,7 @@ class OptionsWidgetsTestCase extends FieldTestCase {
// Display form: with no field data, nothing is checked.
$this->drupalGet('test-entity/' . $entity->ftid .'/edit');
$this->assertNoFieldChecked("edit-card-2-$langcode--0");
$this->assertNoFieldChecked("edit-card-2-$langcode-0");
$this->assertNoFieldChecked("edit-card-2-$langcode-1");
$this->assertNoFieldChecked("edit-card-2-$langcode-2");
$this->assertRaw('Some dangerous &amp; unescaped <strong>markup</strong>', t('Option text was properly filtered.'));
@ -151,7 +151,7 @@ class OptionsWidgetsTestCase extends FieldTestCase {
// Display form: check that the right options are selected.
$this->drupalGet('test-entity/' . $entity->ftid .'/edit');
$this->assertFieldChecked("edit-card-2-$langcode--0");
$this->assertFieldChecked("edit-card-2-$langcode-0");
$this->assertNoFieldChecked("edit-card-2-$langcode-1");
$this->assertFieldChecked("edit-card-2-$langcode-2");
@ -166,7 +166,7 @@ class OptionsWidgetsTestCase extends FieldTestCase {
// Display form: check that the right options are selected.
$this->drupalGet('test-entity/' . $entity->ftid .'/edit');
$this->assertFieldChecked("edit-card-2-$langcode--0");
$this->assertFieldChecked("edit-card-2-$langcode-0");
$this->assertNoFieldChecked("edit-card-2-$langcode-1");
$this->assertNoFieldChecked("edit-card-2-$langcode-2");

View File

@ -94,7 +94,7 @@ class FieldUITestCase extends DrupalWebTestCase {
// should also appear in the 'taxonomy term' entity.
$vocabulary = taxonomy_vocabulary_load(1);
$this->drupalGet('admin/structure/taxonomy/' . $vocabulary->machine_name . '/fields');
$this->assertTrue($this->xpath('//select[@id="edit--add-existing-field-field-name"]//option[@value="' . $this->field_name . '"]'), t('Existing field was found in account settings.'));
$this->assertTrue($this->xpath('//select[@name="_add_existing_field[field_name]"]//option[@value="' . $this->field_name . '"]'), t('Existing field was found in account settings.'));
}
/**

View File

@ -1659,7 +1659,9 @@ class DrupalWebTestCase extends DrupalTestCase {
// http://www.w3.org/TR/html4/interact/forms.html#h-17.13.4.1
$post[$key] = urlencode($key) . '=' . urlencode($value);
}
if ($ajax && isset($submit['triggering_element'])) {
// For AJAX requests, add '_triggering_element_*' and
// 'ajax_html_ids' to the POST data, as ajax.js does.
if ($ajax) {
if (is_array($submit['triggering_element'])) {
// Get the first key/value pair in the array.
$post['_triggering_element_value'] = '_triggering_element_value=' . urlencode(reset($submit['triggering_element']));
@ -1668,6 +1670,10 @@ class DrupalWebTestCase extends DrupalTestCase {
else {
$post['_triggering_element_name'] = '_triggering_element_name=' . urlencode($submit['triggering_element']);
}
foreach ($this->xpath('//*[@id]') as $element) {
$id = (string) $element['id'];
$post[] = urlencode('ajax_html_ids[]') . '=' . urlencode($id);
}
}
$post = implode('&', $post);
}
@ -1698,10 +1704,78 @@ class DrupalWebTestCase extends DrupalTestCase {
}
/**
* Execute a POST request on an AJAX path and JSON decode the result.
* Execute an AJAX submission.
*
* This executes a POST as ajax.js does. It uses the returned JSON data, an
* array of commands, to update $this->content using equivalent DOM
* manipulation as is used by ajax.js. It also returns the array of commands.
*
* @see ajax.js
*/
protected function drupalPostAJAX($path, $edit, $triggering_element, $ajax_path = 'system/ajax', array $options = array(), array $headers = array()) {
return drupal_json_decode($this->drupalPost($path, $edit, array('path' => $ajax_path, 'triggering_element' => $triggering_element), $options, $headers));
protected function drupalPostAJAX($path, $edit, $triggering_element, $ajax_path = 'system/ajax', array $options = array(), array $headers = array(), $form_html_id = NULL, $ajax_settings = array()) {
// Get the content of the initial page prior to calling drupalPost(), since
// drupalPost() replaces $this->content.
if (isset($path)) {
$this->drupalGet($path, $options);
}
$content = $this->content;
$return = drupal_json_decode($this->drupalPost(NULL, $edit, array('path' => $ajax_path, 'triggering_element' => $triggering_element), $options, $headers, $form_html_id));
// We need $ajax_settings['wrapper'] to perform DOM manipulation.
if (!empty($ajax_settings) && !empty($return)) {
// DOM can load HTML soup. But, HTML soup can throw warnings, suppress
// them.
@$dom = DOMDocument::loadHTML($content);
foreach ($return as $command) {
// @todo ajax.js can process commands other than 'insert' and can
// process commands that include a 'selector', but these are hard to
// emulate with DOMDocument. For now, we only implement 'insert'
// commands that use $ajax_settings['wrapper'].
if ($command['command'] == 'insert' && !isset($command['selector'])) {
// $dom->getElementById() doesn't work when drupalPostAJAX() is
// invoked multiple times for a page, so use XPath instead. This also
// sets us up for adding support for $command['selector'], though it
// will require transforming a jQuery selector to XPath.
$xpath = new DOMXPath($dom);
$wrapperNode = $xpath->query('//*[@id="' . $ajax_settings['wrapper'] . '"]')->item(0);
if ($wrapperNode) {
// ajax.js adds an enclosing DIV to work around a Safari bug.
$newDom = DOMDocument::loadHTML('<div>' . $command['data'] . '</div>');
$newNode = $dom->importNode($newDom->documentElement->firstChild->firstChild, TRUE);
$method = isset($command['method']) ? $command['method'] : $ajax_settings['method'];
// The "method" is a jQuery DOM manipulation function. Emulate each
// one using PHP's DOMNode API.
switch ($method) {
case 'replaceWith':
$wrapperNode->parentNode->replaceChild($newNode, $wrapperNode);
break;
case 'append':
$wrapperNode->appendChild($newNode);
break;
case 'prepend':
// If no firstChild, insertBefore() falls back to appendChild().
$wrapperNode->insertBefore($newNode, $wrapperNode->firstChild);
break;
case 'before':
$wrapperNode->parentNode->insertBefore($newNode, $wrapperNode);
break;
case 'after':
// If no nextSibling, insertBefore() falls back to appendChild().
$wrapperNode->parentNode->insertBefore($newNode, $wrapperNode->nextSibling);
break;
case 'html':
foreach ($wrapperNode->childNodes as $childNode) {
$wrapperNode->removeChild($childNode);
}
$wrapperNode->appendChild($newNode);
break;
}
}
}
}
$this->drupalSetContent($dom->saveHTML());
}
return $return;
}
/**
@ -2787,6 +2861,36 @@ class DrupalWebTestCase extends DrupalTestCase {
return $this->assertNoFieldByXPath($this->constructFieldXpath('name', $field) . '|' . $this->constructFieldXpath('id', $field), '', $message, $group);
}
/**
* Assert that each HTML ID is used for just a single element.
*
* @param $message
* Message to display.
* @param $group
* The group this message belongs to.
* @param $ids_to_skip
* An optional array of ids to skip when checking for duplicates. It is
* always a bug to have duplicate HTML IDs, so this parameter is to enable
* incremental fixing of core code. Whenever a test passes this parameter,
* it should add a "todo" comment above the call to this function explaining
* the legacy bug that the test wishes to ignore and including a link to an
* issue that is working to fix that legacy bug.
* @return
* TRUE on pass, FALSE on fail.
*/
protected function assertNoDuplicateIds($message = '', $group = 'Other', $ids_to_skip = array()) {
$status = TRUE;
foreach ($this->xpath('//*[@id]') as $element) {
$id = (string) $element['id'];
if (isset($seen_ids[$id]) && !in_array($id, $ids_to_skip)) {
$this->fail(t('The HTML ID %id is unique.', array('%id' => $id)), $group);
$status = FALSE;
}
$seen_ids[$id] = TRUE;
}
return $this->assert($status, $message, $group);
}
/**
* Helper function: construct an XPath for the given set of attributes and value.
*

View File

@ -2,8 +2,8 @@
// $Id$
class AJAXTestCase extends DrupalWebTestCase {
function setUp() {
parent::setUp('ajax_test', 'ajax_forms_test');
function setUp($modules = array()) {
parent::setUp(array_unique(array_merge(array('ajax_test', 'ajax_forms_test'), $modules)));
}
/**
@ -204,6 +204,91 @@ class AJAXFormValuesTestCase extends AJAXTestCase {
}
}
/**
* Tests that AJAX-enabled forms work when multiple instances of the same form are on a page.
*/
class AJAXMultiFormTestCase extends AJAXTestCase {
public static function getInfo() {
return array(
'name' => 'AJAX multi form',
'description' => 'Tests that AJAX-enabled forms work when multiple instances of the same form are on a page.',
'group' => 'AJAX',
);
}
function setUp() {
parent::setUp(array('form_test'));
// Create a multi-valued field for 'page' nodes to use for AJAX testing.
$field_name = 'field_ajax_test';
$field = array(
'field_name' => $field_name,
'type' => 'text',
'cardinality' => FIELD_CARDINALITY_UNLIMITED,
);
field_create_field($field);
$instance = array(
'field_name' => $field_name,
'entity_type' => 'node',
'bundle' => 'page',
);
field_create_instance($instance);
// Login a user who can create 'page' nodes.
$this->web_user = $this->drupalCreateUser(array('create page content'));
$this->drupalLogin($this->web_user);
}
/**
* Test that a page with the 'page_node_form' included twice works correctly.
*/
function testMultiForm() {
// HTML IDs for elements within the field are potentially modified with
// each AJAX submission, but these variables are stable and help target the
// desired elements.
$field_name = 'field_ajax_test';
$field_xpaths = array(
'page-node-form' => '//form[@id="page-node-form"]//div[contains(@class, "field-name-field-ajax-test")]',
'page-node-form--2' => '//form[@id="page-node-form--2"]//div[contains(@class, "field-name-field-ajax-test")]',
);
$button_name = $field_name . '_add_more';
$button_value = t('Add another item');
$button_xpath_suffix = '//input[@name="' . $button_name . '"]';
$field_items_xpath_suffix = '//input[@type="text"]';
// Ensure the initial page contains both node forms and the correct number
// of field items and "add more" button for the multi-valued field within
// each form.
$this->drupalGet('form-test/two-instances-of-same-form');
foreach ($field_xpaths as $form_id => $field_xpath) {
$this->assert(count($this->xpath($field_xpath . $field_items_xpath_suffix)) == 1, t('Found the correct number of field items on the initial page.'));
$this->assertFieldByXPath($field_xpath . $button_xpath_suffix, NULL, t('Found the "add more" button on the initial page.'));
}
// @todo Legacy bug of duplicate ids for filter-guidelines-FORMAT. See
// http://drupal.org/node/755566.
$this->assertNoDuplicateIds(t('Initial page contains unique IDs'), 'Other', array('filter-guidelines-1', 'filter-guidelines-2', 'filter-guidelines-3'));
// Submit the "add more" button of each form twice. After each corresponding
// page update, ensure the same as above. To successfully implement
// consecutive AJAX submissions, we need to manage $settings as ajax.js
// does for Drupal.settings.
preg_match('/jQuery\.extend\(Drupal\.settings, (.*?)\);/', $this->content, $matches);
$settings = drupal_json_decode($matches[1]);
foreach ($field_xpaths as $form_id => $field_xpath) {
for ($i=0; $i<2; $i++) {
$button = $this->xpath($field_xpath . $button_xpath_suffix);
$button_id = (string) $button[0]['id'];
$commands = $this->drupalPostAJAX(NULL, array(), array($button_name => $button_value), 'system/ajax', array(), array(), $form_id, $settings['ajax'][$button_id]);
$settings = array_merge_recursive($settings, $commands[0]['settings']);
$this->assert(count($this->xpath($field_xpath . $field_items_xpath_suffix)) == $i+2, t('Found the correct number of field items after an AJAX submission.'));
$this->assertFieldByXPath($field_xpath . $button_xpath_suffix, NULL, t('Found the "add more" button after an AJAX submission.'));
// @todo Legacy bug of duplicate ids for filter-guidelines-FORMAT. See
// http://drupal.org/node/755566.
$this->assertNoDuplicateIds(t('Updated page contains unique IDs'), 'Other', array('filter-guidelines-1', 'filter-guidelines-2', 'filter-guidelines-3'));
}
}
}
}
/**
* Miscellaneous AJAX tests using ajax_test module.

View File

@ -748,15 +748,15 @@ class DrupalHTMLIdentifierTestCase extends DrupalUnitTestCase {
$this->assertIdentical(drupal_html_id('invalid,./:@\\^`{Üidentifier'), 'invalididentifier', t('Strip invalid characters.'));
// Verify Drupal coding standards are enforced.
$this->assertIdentical(drupal_html_id('ID NAME_[1]'), 'id-name--1', t('Enforce Drupal coding standards.'));
$this->assertIdentical(drupal_html_id('ID NAME_[1]'), 'id-name-1', t('Enforce Drupal coding standards.'));
// Reset the static cache so we can ensure the unique id count is at zero.
drupal_static_reset('drupal_html_id');
// Clean up IDs with invalid starting characters.
$this->assertIdentical(drupal_html_id('test-unique-id'), 'test-unique-id', t('Test the uniqueness of IDs #1.'));
$this->assertIdentical(drupal_html_id('test-unique-id'), 'test-unique-id-2', t('Test the uniqueness of IDs #2.'));
$this->assertIdentical(drupal_html_id('test-unique-id'), 'test-unique-id-3', t('Test the uniqueness of IDs #3.'));
$this->assertIdentical(drupal_html_id('test-unique-id'), 'test-unique-id--2', t('Test the uniqueness of IDs #2.'));
$this->assertIdentical(drupal_html_id('test-unique-id'), 'test-unique-id--3', t('Test the uniqueness of IDs #3.'));
}
}

View File

@ -813,35 +813,28 @@ class FormsRebuildTestCase extends DrupalWebTestCase {
$this->web_user = $this->drupalCreateUser(array('create page content'));
$this->drupalLogin($this->web_user);
// Get the form for adding a 'page' node. Save the content in a local
// variable, because drupalPostAJAX() will replace $this->content.
// Get the form for adding a 'page' node. Submit an "add another item" AJAX
// submission and verify it worked by ensuring the updated page has two text
// field items in the field for which we just added an item.
$this->drupalGet('node/add/page');
$content = $this->content;
preg_match('/jQuery\.extend\(Drupal\.settings, (.*?)\);/', $this->content, $matches);
$settings = drupal_json_decode($matches[1]);
$button = $this->xpath('//input[@name="field_ajax_test_add_more"]');
$button_id = (string) $button[0]['id'];
$this->drupalPostAJAX(NULL, array(), array('field_ajax_test_add_more' => t('Add another item')), 'system/ajax', array(), array(), 'page-node-form', $settings['ajax'][$button_id]);
$this->assert(count($this->xpath('//div[contains(@class, "field-name-field-ajax-test")]//input[@type="text"]')) == 2, t('AJAX submission succeeded.'));
// Submit an "add another item" AJAX submission and verify it worked by
// ensuring it returned two text fields.
$commands = $this->drupalPostAJAX(NULL, array(), array('field_ajax_test_add_more' => t('Add another item')));
$fragment = simplexml_load_string('<div>' . $commands[1]['data'] . '</div>');
$this->assert(count($fragment->xpath('//input[@type="text"]')) == 2, t('AJAX submission succeeded.'));
// Submit the form with the non-AJAX "Save" button. Leave the title field
// blank to trigger a validation error. Restore $this->content first, as
// drupalPost() needs that to contain the form, not the JSON string left by
// drupalPostAJAX().
// @todo While not necessary for this test, we would be emulating the
// browser better by calling drupalPost() with the AJAX-modified content
// rather than with the original content from the drupalGet(), but that's
// not possible with the current implementation of drupalPostAJAX(). See
// http://drupal.org/node/384992
$this->drupalSetContent($content);
// Submit the form with the non-AJAX "Save" button, leaving the title field
// blank to trigger a validation error, and ensure that a validation error
// occurred, because this test is for testing what happens when a form is
// re-rendered without being re-built, which is what happens when there's
// a validation error.
$this->drupalPost(NULL, array(), t('Save'));
// Ensure that a validation error occurred, since this test is for testing
// what happens to the form action after a validation error.
$this->assertText('Title field is required.', t('Non-AJAX submission correctly triggered a validation error.'));
// Ensure that the form's action is correct.
$this->assertFieldByXPath('//form[@id="page-node-form" and @action="' . url('node/add/page') . '"]', NULL, t('Re-rendered form contains the correct action value.'));
$forms = $this->xpath('//form[contains(@class, "node-page-form")]');
$this->assert(count($forms) == 1 && $forms[0]['action'] == url('node/add/page'), t('Re-rendered form contains the correct action value.'));
}
}

View File

@ -126,6 +126,18 @@ function form_test_menu() {
'type' => MENU_CALLBACK,
);
if (module_exists('node')) {
$items['form-test/two-instances-of-same-form'] = array(
'title' => 'AJAX test with two form instances',
'page callback' => 'form_test_two_instances',
'access callback' => 'node_access',
'access arguments' => array('create', 'page'),
'file path' => drupal_get_path('module', 'node'),
'file' => 'node.pages.inc',
'type' => MENU_CALLBACK,
);
}
return $items;
}
@ -1040,3 +1052,20 @@ function form_test_user_register_form_rebuild($form, &$form_state) {
drupal_set_message('Form rebuilt.');
$form_state['rebuild'] = TRUE;
}
/**
* Menu callback that returns two instances of the node form.
*/
function form_test_two_instances() {
global $user;
$node1 = (object) array(
'uid' => $user->uid,
'name' => (isset($user->name) ? $user->name : ''),
'type' => 'page',
'language' => LANGUAGE_NONE,
);
$node2 = clone($node1);
$return['node_form_1'] = drupal_get_form('page_node_form', $node1);
$return['node_form_2'] = drupal_get_form('page_node_form', $node2);
return $return;
}