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.
Comment | File | Size | Author |
---|---|---|---|
#13 | book_parent_select_form_id_1.patch | 1.51 KB | AaronBauman |
#11 | book_parent_select_form_id.patch | 1.51 KB | AaronBauman |
#6 | book_parent_select_form_id.patch | 927 bytes | edmund.kwok |
Comments
Comment #1
beginner CreditAttribution: beginner commentedYou can see the same problem more easily with firebug:
Comment #2
beginner CreditAttribution: beginner commentedI 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. :~?
This is beyond me.
The stop gap solution for me is to do the following, which is far from ideal:
I guess the bug is in D7, too.
Apparently, it is a bug in the AHAH implementation of FAPI.
Comment #3
beginner CreditAttribution: beginner commentedComment #4
pwolanin CreditAttribution: pwolanin commentedis it only twice? At one point early on the code keep adding nested divs
Comment #5
beginner CreditAttribution: beginner commentedIt 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.
Comment #6
edmund.kwok CreditAttribution: edmund.kwok commentedThe 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.
Comment #7
lilou CreditAttribution: lilou commentedLook good and all tests pass (thx Testbed ;-)
Comment #8
Dries CreditAttribution: Dries commentedIt would be good to understand if renaming is a good thing. Removing it might also be an option. Who knows? :)
Comment #9
pwolanin CreditAttribution: pwolanin commented@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.
Comment #11
AaronBaumanI 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.
Comment #13
AaronBaumanfixed form properties
Comment #14
thedavidmeister CreditAttribution: thedavidmeister commentedI 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.