book.module has some xhtml validation errors created by the ahah callback.

Error Line 107, Column 95: ID "EDIT-BOOK-PLID-WRAPPER" already defined.
<div class="form-item" id="edit-book-plid-wrapper">

To reproduce:

Use Firefox with the "Web Developer" 1.1.5 extension.
On a fresh Drupal-6 install, enable book module. Create one book node.
Create a second book node: on the node form expand the book outline fieldset, select the other book node to add to that book.
Now, without submitting, look at the Web Developer toolbar -> View Source -> View generated source. Save the source in a file, and upload to the web validator: http://validator.w3.org/#validate_by_upload

This seems like a minor problem, but it is causing me a lot of headaches right now.

The ahah callback in book.module is supposed to replace the content of the element with the ID edit-book-plid-wrapper, and since there are now two of them, different browsers interpret the DOM differently.

This validation error breaks my module which is adding some fields to the ahah callback. Konquerer seems to work well with my module, but Firefox is broken. Since to start with the html is invalid, different browsers handle the extra fields differently, breaking the ahah effects.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

beginner’s picture

You can see the same problem more easily with firebug:

<div id="edit-book-bid-wrapper" class="form-item">
</div>
<div id="edit-book-plid-wrapper">
<div style="display: block;">
<div id="edit-book-plid-wrapper" class="form-item">
</div>
</div>
</div>
beginner’s picture

Version: 6.x-dev » 7.x-dev

I tried debugging this thing, but it's beyond my level of understanding of FAPI.

In the function book_form_update() (which is the ahah callback), there is a call to drupal_render().
If I do dpm($form['book']['plid']) just before, the form does not include the said ID, but if I do dpm($output) just after, the ID is there. :~?

      // Build and render the new select element, then return it in JSON format.
      $form_state = array();
      $form['#post'] = array();
      $form = form_builder($form['form_id']['#value'] , $form, $form_state);
      dpm($form['book']['plid'], 'book plid pre render'); // --> There is no value "edit-book-plid-wrapper" anywhere here.
      $output = drupal_render($form['book']['plid']);
      dpm($output, 'output'); // --> this includes the superfluous ID: <div class="form-item" id="edit-book-plid-wrapper">
      drupal_json(array('status' => TRUE, 'data' => $output));

This is beyond me.

The stop gap solution for me is to do the following, which is far from ideal:

      $output = drupal_render($form['book']['plid']);
      $output = preg_replace('/edit-book-plid-wrapper/', 'edit-book-plid-wrapper-bis', $output);

I guess the bug is in D7, too.
Apparently, it is a bug in the AHAH implementation of FAPI.

beginner’s picture

Title: book: same ID referenced twice with AHAH » FAPI / AHAH: same ID referenced twice in book.module.
pwolanin’s picture

is it only twice? At one point early on the code keep adding nested divs

beginner’s picture

It kept adding divs *because* of the duplicate ID. The exact behavior would depend on the browser: when asked to take an action on a specific ID, and they have two elements with the same ID, they have to make a choice as to which to apply the action to.

My code seemed to work with Konqueror, because the ahah action was performed on the first element with the same ID, which encompassed the second one (which therefore was removed and replaced by the updated code).
However the same code was broken with Firefox, because it took the second element, giving the impression that the new form elements kept being added and never removed.

The bug is in the AHAH implementation: jQuery doesn't seem to be removing the original element with the wrapper ID, but FAPI returns another element with the same ID with each AHAH action.

edmund.kwok’s picture

Status: Active » Needs review
FileSize
927 bytes

The parent select form is enclosed in div#edit-book-plid-wrapper (look in _book_parent_select); since the select id is $form['book']['plid'], the id of the wrapper generated is also called "edit-book-plid-wrapper" which is why the div id duplicates.

One solution is to rename the first wrapper; attached patch renames the wrapper to edit-book-plid-form-wrapper. Not sure what the convention in core is; can't find a precedence for -form-wrapper suffixes.

lilou’s picture

Status: Needs review » Reviewed & tested by the community

Look good and all tests pass (thx Testbed ;-)

Dries’s picture

It would be good to understand if renaming is a good thing. Removing it might also be an option. Who knows? :)

pwolanin’s picture

@Dries - since the AHAH wrapper is being manually specified, this looks like a reasonable solution - probably it was only an oversight initally that lead to the duplicate name.

Status: Reviewed & tested by the community » Needs work

The last submitted patch failed testing.

AaronBauman’s picture

Status: Needs work » Needs review
FileSize
1.51 KB

I tested this and found that though the DOM validation issue was resolved, the ajax callback was not behaving as expected.

When you follow the steps in the OP to create the DOM validation issue, everything goes ok.
But then, when you change the book dropdown back to "create new book" or "none", the ajax callback fails to update the DOM because the DOM element div#edit-book-plid-form-wrapper was replaced with an anonymous div (ie. a div with no "id" attribute).

The reason is that when returning the list of available books, _book_parent_select() does not include the expected wrapper.

The fix (attached patch) is to make sure that _book_parent_select() always returns a form with a #prefix attribute that includes the wrapper, instead of overwriting it.

Otherwise, rerolled @ HEAD.

Status: Needs review » Needs work

The last submitted patch, book_parent_select_form_id.patch, failed testing.

AaronBauman’s picture

Status: Needs work » Needs review
FileSize
1.51 KB

fixed form properties

thedavidmeister’s picture

Version: 7.x-dev » 6.x-dev
Issue summary: View changes
Status: Needs review » Needs work

I can reproduce this bug in D6 by following the instructions in the OP.

I cannot reproduce this bug in D7 or D8, even though the code in addParentSelectFormElements() looks similar to the D7 code and previous reports stated the bug exists in D7.

This patch does not apply against D6 and would need to be re-rolled for git anyway.

#2094585: [policy, no patch] Core review bonus for #2181941: Drupal should recognise $_SERVER['HTTP_X_FORWARDED_PROTO'] when attempting to detect a https request.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.