Skip to content

Fix uncheck checkbox and submit input type hidden value#36

Merged
Naktibalda merged 9 commits intoCodeception:masterfrom
SOHELAHMED7:fix-uncheck-and-send-input-type-hidden-value
Apr 17, 2021
Merged

Fix uncheck checkbox and submit input type hidden value#36
Naktibalda merged 9 commits intoCodeception:masterfrom
SOHELAHMED7:fix-uncheck-and-send-input-type-hidden-value

Conversation

@SOHELAHMED7
Copy link
Copy Markdown
Contributor

@SOHELAHMED7 SOHELAHMED7 commented Apr 9, 2021

Current status

When a form have following inputs:

<input type="hidden" name="coffee" value="0">
<input type="checkbox" name="coffee" value="1" id="coffee-id" checked>

and when checkbox is unchecked and then if the form is submitted, value '0' of coffee is not sent.

Ideally it should.

This PR fix that issue.

Failing test: d56484c

@SamMousa

Have any questions, feel free to ask

Comment thread src/Codeception/Lib/InnerBrowser.php Outdated
Comment thread src/Codeception/Lib/InnerBrowser.php Outdated
@Naktibalda
Copy link
Copy Markdown
Member

Wouldn't it be better to check if Checkbox field is unchecked and then skip it.

Browsers process form fields in a linear fashion (that's the point of putting hidden field before checkbox).
Your patch introduces non-linear behaviour which is nothing like browser's and it will introduce a lot new bugs.

Could you check if skipping unchecked fields work?

Before:

foreach ($fields as $field) {
	if ($field instanceof FileFormField || $field->isDisabled() || !$field->hasValue()) {
	    continue;
	}
	// ...
}

After:

foreach ($fields as $field) {
	if ($field instanceof FileFormField || $field->isDisabled() || !$field->hasValue()) {
	    continue;
	}
	
	if ($field instanceof CheckboxField && $field->isChecked() === false) {
	    continue;
	}
	// ...
}

@SamMousa
Copy link
Copy Markdown
Contributor

The issue is that the symfony component does not add all fields to the form... It essentially deduplicates based on name too early

@Naktibalda
Copy link
Copy Markdown
Member

Ah, it is a WONTFIX issue: symfony/symfony#11689

@SamMousa
Copy link
Copy Markdown
Contributor

Yeah, so the question is do we want to work around it via the solution proposed by @SOHELAHMED7 ?

@Naktibalda
Copy link
Copy Markdown
Member

It seems that a better solution would be to stop using Symfony\Component\DomCrawler\Form, extract all fields using Crawler and do all processing in InnerBrowser code, but it is a lot of work.

So a simple fix like this will have to do for now.

Comment thread src/Codeception/Lib/InnerBrowser.php
Comment thread src/Codeception/Lib/InnerBrowser.php
@SOHELAHMED7
Copy link
Copy Markdown
Contributor Author

@Naktibalda

Do you mean to say like this 0d03718 ?

Comment thread src/Codeception/Lib/InnerBrowser.php
@Naktibalda Naktibalda merged commit d5b87df into Codeception:master Apr 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants