Mainly some code cleaning and better use of jQuery (.click and .val). Removed choiceDIV which isn't used. The new choices are also inserterted at the correct place in the DOM by using insertBefore("#morechoices"). #morechoises is also hidden with $("#morechoices").hide();.

While doing this I remebered that you (Chris) said that this worked for Drupal 4.7 but not 5. Might it be due to some security improvements made in the formAPI? But on the other hand, they should have been backported to 4.7... Any idea?

CommentFileSizeAuthor
advpoll_addchoices.patch2.41 KBanders.fajerson

Comments

anders.fajerson’s picture

anders.fajerson commented

I forgot to say that I also remove the code relating to // Be sure not to add it more than once since it isn't needed. The code is only added once when you use $(document).ready

ChrisKennedy’s picture

ChrisKennedy commented

Nice work, you clearly have a much better understanding of the jQuery API than I do. However, the $(document).ready check is kind of needed - try previewing and you'll see the button twice. This is probably due to the #multistep flag though, and I haven't been able to find any documentation on it so far.

Yeah, my suspicion is that security and/or validation improvements may be causing the problem in 5.0, but I'm not sure.

ChrisKennedy’s picture

ChrisKennedy commented
Status: Needs review » Fixed

I've found a workaround by using $_POST. I tried various combinations of form_values and #DANGEROUS_SKIP_CHECK (see form.inc source code) but couldn't find any other working method.

http://drupal.org/cvs?commit=45218

anders.fajerson’s picture

anders.fajerson commented

I didn't expect the code to be added twice, but I see it's due to the #multistep flag. Anyway it's much better to handle that within the php code, so some good came out of it. Great.

I had an idea to further improve the javascript by removing the dependency on the translate function. Simply the idea was to copy an existing choice form item and replace the choice number with the relevant "nextChoiceN". This would also mean that we could include the javascript in an external file instead, which is a better option. I run into some problems though, maybe I give it another shot later on.

ChrisKennedy’s picture

ChrisKennedy commented

Ah that is a good idea, then we could just include the js file in _menu.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)