Didn't actually check, but #322344: (notice) Form improvements from Views most probably broke JS 'add more' button (no more $form['#post'])

Work is under way to refactor this area in #367567: Use AJAX framework for "Add more" / load include file containing form / introduce $form_state['build_info'], but we need to unbreak the current implementation first. No time for this tonight :-)

CommentFileSizeAuthor
#4 field_fix_add_more-402264-5.patch6.86 KByched
#2 field_fix_add_more-402264-3.patch6.41 KByched
#1 field_fix_add_more-402264-2.patch1.44 KByched

Comments

yched’s picture

yched commented
Status: Active » Needs review
StatusFileSize
new field_fix_add_more-402264-2.patch1.44 KB

Here's a hotfix patch. As I wrote above, this area needs serious refactoring, but we need to fix the current code first, at least to have something to compare against.

yched’s picture

yched commented
Title: adapt field's 'add-more' JS handler to recent FAPI changes » adapt field's 'add-more' JS handler to recent FAPI changes, + test
StatusFileSize
new field_fix_add_more-402264-3.patch6.41 KB

Also : If we had a test for the AHAH-'add more' button, which I've been willing to write for some time now, then it would have been the job of the offending patch to fix this :-p.
So, same fix as above, with a test for the AHAH behavior. It will also be useful to validate #367567: Use AJAX framework for "Add more" / load include file containing form / introduce $form_state['build_info'],

The test steals the code poll.test uses to test its own 'add more' button, and abstracts it to a separate _fieldPostAhah() method.
Not sure whether it's worth promoting to DrupalWebTestCase, I leave that out to more FAPI / test oriented people to decide - in a separate thread ?

The test also updates the regexp used in the existing 'non AHAH' test to a more specific one, to make it match the AHAH test.

karens’s picture

karens commented

I found one more instance of '#programmed' on line 188 of field.form.inc that needs to be changed to 'programmed' to match the new API. Other than that the patch works fine. The bot doesn't like my patches, so I haven't re-rolled it myself, but if someone could add that fix in it should be good to go.

yched’s picture

yched commented
StatusFileSize
new field_fix_add_more-402264-5.patch6.86 KB

Right, New patch with that last '#programmed' bit fixed.
Setting to RTBC based on Karen's feedback.

yched’s picture

yched commented
Status: Needs review » Reviewed & tested by the community

'Setting to RTBC', he said.

cburschka’s picture

cburschka
they
commented 
$pattern[$weight] = "<input [^>]*value=\"$value\" [^>]*";

I'm not sure, but this looks odd compared to other places where we do string concatenation. Shouldn't you do '<input [^>]*value="' . $value . '" [^>]*'?

However, this seems to still apply, and it's been sitting in the RTBC queue for a month, so I'm bumping it up.

yched’s picture

yched commented

Hm, I'm not sure I see why this wouldn't be correct ?

bjaspan’s picture

bjaspan commented

I don't think we have a ban on double-quoted string interpolation. Leaving as RTBC. Angie will let us know if it is a problem.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

I've committed this. We can continue to work on this if necessary. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.