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] exclude fields inside "template" tags #28147

Merged
merged 1 commit into from Nov 26, 2018
Merged

[DomCrawler] exclude fields inside "template" tags #28147

merged 1 commit into from Nov 26, 2018

Conversation

Gorjunov
Copy link

@Gorjunov Gorjunov commented Aug 6, 2018

Q A
Branch? 2.8
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #27285
License MIT
Doc PR

Exclude fields values/files if fields are inside template tag. I think better to exclude values only instead of excluding fields at all (described in ticket #27285)

@jakzal
Copy link
Contributor

jakzal commented Aug 15, 2018

I think better to exclude values only instead of excluding fields at all

Could you elaborate on why it's better?

@Gorjunov
Copy link
Author

In this case it represents browser's behavior - element is accessible (e.g. by JS), but server will not get values on form submit. (if I understand right that Form:: getValues() is same as form submit in browser)

@nicolas-grekas
Copy link
Member

The way this is currently implemented could be understood as a new feature because it's adding a new method. If we can do without the new method, that'd be better to qualify as bugfix.

better to exclude values only instead of excluding fields at all

Shouldn't the $fields be not populated with tags inside template instead?

@stof
Copy link
Member

stof commented Sep 19, 2018

this should indeed be implemented when populating the list of fields, as the form in the DOM does not have this field in this case in a browser. It is not only about the submission skipping the values.

@nicolas-grekas nicolas-grekas changed the title [DomCrawler] exclude fields from Form::getValues, Form::getFiles if field is inside template tag [DomCrawler] exclude fields inside "template" tags Nov 24, 2018
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

(implementation updated to filter fields inside <template> using xpath)

@stof
Copy link
Member

stof commented Nov 24, 2018

Tests should also be updated to test that the Form does not contain the corresponding field at all (has returns false for this field name)

@nicolas-grekas
Copy link
Member

@stof test added

@fabpot
Copy link
Member

fabpot commented Nov 26, 2018

Thank you @Gorjunov.

@fabpot fabpot merged commit 19e3e15 into symfony:2.8 Nov 26, 2018
fabpot added a commit that referenced this pull request Nov 26, 2018
This PR was merged into the 2.8 branch.

Discussion
----------

[DomCrawler] exclude fields inside "template" tags

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27285
| License       | MIT
| Doc PR        |

Exclude fields values/files if fields are inside template tag. I think better to exclude values only instead of excluding fields at all (described in ticket #27285)

Commits
-------

19e3e15 [DomCrawler] exclude fields inside "template" tags
@Gorjunov Gorjunov deleted the dom_crawler_exclude_template branch November 26, 2018 19:38
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.

None yet

6 participants