8000 [DomCrawler] Fails in PHP7.2 for same name fields, different types · Issue #28179 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DomCrawler] Fails in PHP7.2 for same name fields, different types #28179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
michsk opened this issue Aug 9, 2018 · 3 comments
Closed

[DomCrawler] Fails in PHP7.2 for same name fields, different types #28179

michsk opened this issue Aug 9, 2018 · 3 comments
Labels
Bug DomCrawler Status: Needs Review

Comments

@michsk
Copy link
michsk commented Aug 9, 2018 •
edited by javiereguiluz
Loading

Symfony version(s) affected: x.y.z
"name": "symfony/dom-crawler",
"version": "v2.7.49",

Description
Codeception/Codeception#5119
#11689
#11692

<form id="form" action="/index.php" method="post">
<input type="text" id="originator" class="alpha form-control" name="form[origin]" value="thisshouldbeinpost" placeholder="Afzender" maxLength="18">
<select id="originator" name="form[origin]" disabled>
<option value="one">one</option>
<option value="two">two</option>
<option value="tree">tree</option>
</select>
<input type="submit">
</form>

So it seems that the fix mentioned above does not work for PHP7.2. In the above example. The field of the disabled select is chosen over the input field. This is not what PHP would do. Because the field is disabled, the POST does not contain the field.

Possible Solution
Investigate on how to allow different types to be used as well in \Symfony\Component\DomCrawler\Form::addField
@michsk michsk changed the title [DomCrawler] Fails in PHP7.2 for same name fields [DomCrawler] Fails in PHP7.2 for same name fields, different types Aug 9, 2018
@michsk
Copy link
Author
michsk commented Aug 9, 2018

Could it be the order of nodes is changed? Because im amazed that this did work in PHP7.0. I looked at the changelog for php but couldnt find anything. What i could imagine is that the order changed, and that before we had the select first, and than the input. Which would make the input override the select.

@michsk
Copy link
Author
michsk commented Aug 9, 2018 •
edited
Loading

I think we should just exclude disabled form elements when searching for form elements

@michsk
Copy link
Author
michsk commented Aug 9, 2018 •
edited
Loading

So changing
vendor/symfony/dom-crawler/Form.php:456

to ignore the disabled fields. Just like PHP would do, and just like we do when requesting the values in getValues. Fixes this issue. Since the correct input value is not overwritten with the disabled field.

        if (!$node->hasAttribute('name') || !$node->getAttribute('name') || $node->hasAttribute('disabled')) {
            return;
        }

Guess this would also mean we could dump the $field->isDisabled() in public function getValues(), since we don't use the field anymore.

@michsk michsk mentioned this issue Aug 9, 2018
Closed
@sbogx sbogx mentioned this issue Oct 22, 2019
Merged
@fabpot fabpot closed this as completed Feb 3, 2020
fabpot added a commit that referenced this issue Feb 3, 2020
@fabpot
bug #34059 [DomCrawler] Skip disabled fields processing in Form (sbogx)
6c96706 
This PR was merged into the 3.4 branch.

Discussion
----------

[DomCrawler] Skip disabled fields processing in Form

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #28179
| License       | MIT

Commits
-------

c73b042 bug #28179 [DomCrawler] Skip disabled fields processing in Form
nicolas-grekas added a commit that referenced this issue Feb 3, 2020
@nicolas-grekas
Merge branch '3.4' into 4.4
ed7bb82 
* 3.4:
  [Phpunit] Fix running skipped tests expecting only deprecations
  [DependencyInjection] #35505 Fix typo in test name
  [Yaml][Inline] Fail properly on empty object tag and empty const tag
  Check non-null type for numeric type
  Check value isset to avoid PHP notice
  bug #28179 [DomCrawler] Skip disabled fields processing in Form
chalasr added a commit that referenced this issue Feb 3, 2020
@chalasr
Merge branch '4.4' into 5.0
5da1bcf 
* 4.4:
  [Phpunit] Fix running skipped tests expecting only deprecations
  Fix merge
  [Config] dont catch instances of Error
  [HttpClient] fix HttpClientDataCollector when handling canceled responses
  [DependencyInjection] #35505 Fix typo in test name
  [Yaml][Inline] Fail properly on empty object tag and empty const tag
  Check non-null type for numeric type
  Check value isset to avoid PHP notice
  bug #28179 [DomCrawler] Skip disabled fields processing in Form
chalasr added a commit that referenced this issue Feb 3, 2020
@chalasr
Merge branch '5.0'
12ca646 
* 5.0:
  [Phpunit] Fix running skipped tests expecting only deprecations
  Fix merge
  [Config] dont catch instances of Error
  [HttpClient] fix HttpClientDataCollector when handling canceled responses
  [FrameworkBundle] remove mention of the old Controller class
  [DependencyInjection] #35505 Fix typo in test name
  [Yaml][Inline] Fail properly on empty object tag and empty const tag
  Check non-null type for numeric type
  Check value isset to avoid PHP notice
  bug #28179 [DomCrawler] Skip disabled fields processing in Form
dmaicher added a commit to dmaicher/symfony that referenced this issue Mar 3, 2020
@dmaicher
Revert "bug symfony#28179 [DomCrawler] Skip disabled fields processin…
af17f5a 
…g in Form"

This reverts commit c73b042.
@dmaicher dmaicher mentioned this issue Mar 3, 2020
Closed
fabpot added a commit that referenced this issue Mar 3, 2020
@fabpot
bug #35937 Revert "bug #28179 [DomCrawler] Skip disabled fields proce…
3a8da96 
…ssing in Form" (dmaicher)

This PR was merged into the 3.4 branch.

Discussion
----------

Revert "bug #28179 [DomCrawler] Skip disabled fields processing in Form"

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #35923
| License       | MIT
| Doc PR        | -

Reverts #34059

See #35923

Commits
-------

af17f5a Revert "bug #28179 [DomCrawler] Skip disabled fields processing in Form"
nicolas-grekas added a commit that referenced this issue Mar 11, 2020
@nicolas-grekas
Merge branch '3.4' into 4.4
d0b7445 
* 3.4:
  [Yaml] fix dumping strings containing CRs
  [DI] Fix XmlFileLoader bad error message
  Tweak message
  improve PlaintextPasswordEncoder docBlock summary
  [Validator] Add two missing translations for the Arabic (ar) locale
  Use some PHP 5.4 constants unconditionally
  Revert "bug #28179 [DomCrawler] Skip disabled fields processing in Form"
  Add Spanish translation
  Fix typo
  [Validator] add Japanese translation
  Fix typo
  Add Polish translation
  [SecurityBundle] Minor fixes in configuration tree builder
  bumped Symfony version to 3.4.39
  updated VERSION for 3.4.38
  update CONTRIBUTORS for 3.4.38
  updated CHANGELOG for 3.4.38
This was referenced Mar 27, 2020
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug DomCrawler Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fabpot @xabbuh @michsk @carsonbot
0