Poll does not respect weight in Preview or More button

I've mixed a fix for this into #757154-36: Base form_id via hook_forms() not taken into account for #validate, #submit, hook_form_FORMID_alter()

Extracted.

Reviews, tests, anyone?

This patch may be a prerequisite for aforementioned issue.

Extracting the actual bugs into required assertions:

1. Enter a poll title, two choices, re-order the choices, and add "more choices". The original order of choices is lost.
2. Re-order the choices and click "preview". The order is lost in the form and in the preview.
3. A user without administrative permissions should not be able to enter/alter poll vote counts in the node form.

Alright, technically this should fail.

And, technically, this one should pass. ;)

Interesting. The preview indeed seems to need some more work :)

#7: drupal.poll-choice-weight-access.7.patch queued for re-testing.

wow, we badly need to train people in PHP data types a bit more. :(

Weird aggregator tests.

Now someone needs to make the weights in poll previews work.

#13: drupal.poll-choice-weight-access.13.patch queued for re-testing.

Re-rolled against HEAD.

This one should pass.

Wow, that is quite a patch for such a simpleish sounding issue title. :) Could we maybe get a summary of what's going on and how we got roped into Aggregator and other things?

Heh, yarrr :)

A summary of bugs this patch fixes is in #4.

However, in order to write tests for this, I had to change the function/method signature of assertFieldByXPath() and the corresponding assertNoFieldByXPath() to accept a $value of NULL that really means NULL/nothing. Currently, the $value argument basically defaults to an empty string, but an empty string is a valid value of an input field; i.e.,

<input type="text" value="" />

Therefore, it's impossible to actually assert an empty string as value currently. Thus, I had to update a few test assertions throughout core to not pass totally weird values for $value (such as '', FALSE, and whatnot) when they really meant to pass NULL to completely skip the additional assertion of the field/input's value.

Ah, ok. Thanks for the context. That means this involves an API change for test authors. We might not have a choice, though. Hm.

@quicksketch or anyone else: Could you review and possibly sign off this patch?

To summarize the bugs that this patch fixes once again:

1. Enter a poll title, two choices, re-order the choices, and add "more choices". The original order of choices is lost.
2. Re-order the choices and click "preview". The order is lost in the form and in the preview.
3. A user without administrative permissions should not be able to enter/alter poll vote counts in the node form. The corresponding "vote count" column should also not be visible.

as well as

4. assertFieldByXPath() and assertNoFieldByXPath() do not allow to assert a field with a value attribute of '' (empty string), which is required to make the added tests for Poll work.

mmm, if anyone would use and install Poll module, we'd see much more traction on this issue. :(

quicksketch, anyone, any feedback?

Actually, if you look at the current user interface, then you see that the very basic UI of Poll module's node form is completely b0rked. This must be fixed before release, so I think that the "Release blocker" tag is justified here. (At least we talked about the idea of solidly using this tag this way; feel free to revert, please.)

I can't help. Asked several times and waited for reviews, even sent a tweet about this issue, but nothing.

Intensively and manually tested this patch myself, in addition to the automated tests that have been added.

Thanks for trying, sun. Poor, decrepit, neglected Poll module. :(

Committed to HEAD. It can't be worse than it already is. :P

What's the API change here? Can you describe it? Does it need to be announced?

I don't think so, really. It's a change to assertFieldByXPath() and assertNoFieldByXPath() signature. Only affects test authors doing weird things.