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.

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

[DomCrawler] Add support for formaction and formmethod attributes #20467

Merged
merged 1 commit into from
Dec 2, 2016

Conversation

stof
Copy link
Member

@stof stof commented Nov 9, 2016

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

This adds supports for the formaction and formmethod of submit elements, which override the values defined on the <form> element.
This works only when you call $crawler->form() on a Crawler containing a button, not when it contains the <form> itself of course (as the button override is applied only when using this button to submit, not when using another way).

Other button-level overrides are not implemented:

  • formtarget is useless as we don't implement target either (the Crawler does not deal with frame-based pages anyway)
  • formnovalidate is ignored, as we don't automatically disable the form validation on novalidate either, but we require an explicit disabling instead (this might be subject to a separate PR though, as it could make sense)
  • formenctype is ignored as we also ignore enctype (we always submit file fields, even when missing the proper enctype)

public function testGetUriWithActionOverride()
{
$form = $this->createForm('<form action="/foo"><button type="submit" formaction="/bar" /></form>', null, 'http://localhost/foo/');
$this->assertEquals('http://localhost/bar', $form->getUri(), '->getUri() returns absolute URIs');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you are testing getUri() but in your PR you have changed getRawUri(), right?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe because getRawUri is a protected method (can't be tested directly), which is called by public getUri method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed. getUri (defined in the parent class) calls getRawUri and then resolve the URI to account for non-absolute ones (like in this test)

{
$form = $this->createForm('<form action="/foo"><button type="submit" formaction="/bar" /></form>', null, 'http://localhost/foo/');
$this->assertEquals('http://localhost/bar', $form->getUri(), '->getUri() returns absolute URIs');

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra blank line should be removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@javiereguiluz
Copy link
Member

👍

Status: reviewed

@fabpot
Copy link
Member

fabpot commented Dec 2, 2016

Thank you @stof.

@fabpot fabpot closed this Dec 2, 2016
@fabpot fabpot merged commit 717cf8a into symfony:master Dec 2, 2016
fabpot added a commit that referenced this pull request Dec 2, 2016
… attributes (stof)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DomCrawler] Add support for formaction and formmethod attributes

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

This adds supports for the ``formaction`` and ``formmethod`` of submit elements, which override the values defined on the ``<form>`` element.
This works only when you call ``$crawler->form()`` on a Crawler containing a button, not when it contains the ``<form>`` itself of course (as the button override is applied only when using this button to submit, not when using another way).

Other button-level overrides are not implemented:
- ``formtarget`` is useless as we don't implement ``target`` either (the Crawler does not deal with frame-based pages anyway)
- ``formnovalidate`` is ignored, as we don't automatically disable the form validation on ``novalidate`` either, but we require an explicit disabling instead (this might be subject to a separate PR though, as it could make sense)
- ``formenctype`` is ignored as we also ignore ``enctype`` (we always submit file fields, even when missing the proper enctype)

Commits
-------

717cf8a [DomCrawler] Add support for formaction and formmethod attributes
@stof stof deleted the html5_form branch December 2, 2016 13:11
@nicolas-grekas nicolas-grekas modified the milestones: 3.x, 3.3 Mar 24, 2017
@fabpot fabpot mentioned this pull request May 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants