Test framework does not correctly emulate browser submission of disabled elements [#712990]

I discovered this while working on #426056: Server-side enforcement of #disabled is inconsistent, but while that issue isn't critical, this one is. This fixes the DrupalWebTestCase::handleForm() function to not submit input for disabled elements (since browsers don't). Doing so results in test failures, and this patch fixes the code in form.inc necessary to make those tests pass. It also allows the assertion in the FAPI test to be strengthened. This should go in before #426056: Server-side enforcement of #disabled is inconsistent, and then I'll re-roll that patch.

Comment	File	Size	Author
#3	form_disabled_fixes.3.patch	6.8 KB	effulgentsia
	form_disabled_fixes.patch	5.37 KB	effulgentsia

Comments

Comment #1

12 February 2010 at 21:50

Status:

Needs review

» Needs work

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

Comment #2

eric_a commented 12 February 2010 at 22:44

I can't do a proper review right now but I wanted to say this: for the checkbox and select element the NULL value in $input has a special meaning. This is only relevant for enabled elements but I think it would make all of the code in these functions much easier to understand if there were explicit checks for NULL rather than doing all this isset() stuff when $input !== FALSE.

Comment #3

effulgentsia commented 12 February 2010 at 23:49

Status:

Needs work

» Needs review

Status	File	Size
new	form_disabled_fixes.3.patch	6.8 KB

This fixes the translation test, which was attempting to submit a form with input for a disabled checkbox, correctly causing a failure with the fixed handleForm() function.

This is only relevant for enabled elements but I think it would make all of the code in these functions much easier to understand if there were explicit checks for NULL rather than doing all this isset() stuff when $input !== FALSE.

I think I agree with this, but it's a separate issue, and one worth tackling after #426056: Server-side enforcement of #disabled is inconsistent, since that will decouple #disabled from $input=NULL. This patch doesn't add any new isset($input) into the mix. The only scope of this patch is to fix the test framework to not submit input for disabled elements, and to fix the form.inc code needed to make existing tests pass. The code added to form_type_select_value() follows the same structure as what's already in HEAD for form_type_checkbox_value().

As an aside, these *_value() functions are also hard to read because $input=FALSE means "return the default value if it's different from #default_value". In #622922-164: User input keeping during multistep forms, chx had written some great code to refactor this into *_default_value() functions, but that got removed from that issue in order to keep that issue focused on the problem at hand. After some of these other FAPI issues land, I'll open an issue to re-introduce this value callback / default value callback refactoring, but I have doubts that that will be accepted for D7.