Skip to content
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

[DomCrawler] disabled form fields not reachable anymore #35923

Closed
dmaicher opened this issue Mar 2, 2020 · 5 comments
Closed

[DomCrawler] disabled form fields not reachable anymore #35923

dmaicher opened this issue Mar 2, 2020 · 5 comments
Labels
Bug DomCrawler Status: Needs Review

Comments

@dmaicher
Copy link
Contributor

dmaicher commented Mar 2, 2020 •
edited

Symfony version(s) affected: 3.4.38, 4.4.5, 5.0.5

Description
It seems since #34059 a disabled form field is not added to the form anymore at all which seems odd.

In one of my tests I was retrieving a disabled form field and checking its value. This is not possible anymore now.

How to reproduce

<?php

use Symfony\Component\DomCrawler\Crawler;

$html = <<<HTML
<html>
    <form>
        <input type="text" name="foo" value="bar" disabled />
    </form>
</html>
HTML;

$crawler = new Crawler($html, 'http://localhost/');

dump($crawler->filter('form')->form()->get('foo')->getValue());

Works with 4.4.4 and breaks with 4.4.5: InvalidArgumentException: Unreachable field "foo"
@dmaicher dmaicher added the Bug label Mar 2, 2020
@alexpott
Copy link
Contributor

alexpott commented Mar 3, 2020

The change in #34059 has broken the Drupal test suite too.

We have code like

    $this->assertSession()->checkboxChecked('options[revision]');
    $this->assertSession()->fieldDisabled('options[revision]');

The call to checkboxChecked ends up throwing an error InvalidArgumentException: Unreachable field "revision"

I think that the change in #34059 is too broad.

@alexpott alexpott mentioned this issue Mar 3, 2020
Merged
@nicolas-grekas
Copy link
Member

We should revert #34059 for sure.

I think that the change in #34059 is too broad.

"too broad" => what's the reduced scope you've in mind? Up for a PR?

@dmaicher dmaicher mentioned this issue Mar 3, 2020
Merged
@dmaicher
Copy link
Contributor Author

dmaicher commented Mar 3, 2020

#35937 reverts it for now until there is a "better" fix for #28179

@fabpot fabpot closed this as completed Mar 3, 2020
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"
@VincentLanglet
Copy link
Contributor

Shouldn't a 4.4.6 be released to fix the BC-break ?

@dmaicher
Copy link
Contributor Author

dmaicher commented Mar 9, 2020

Shouldn't a 4.4.6 be released to fix the BC-break ?

There will be a new monthly patch release in 3 weeks or so. Possibly earlier... not sure.

In the meantime just fix symfony/dom-crawler to 4.4.4 😉

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
7 participants
@fabpot @nicolas-grekas @alexpott @dmaicher @xabbuh @VincentLanglet @carsonbot