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

Fix a web profiler form issue with fields added to the form after the form was built #13166

Merged
merged 1 commit into from Dec 30, 2014

Conversation

jakzal
Copy link
Contributor

@jakzal jakzal commented Dec 30, 2014

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? no, but not because of this PR
Fixed tickets #13155
License MIT
Doc PR -

This also make the check consistent with a similar check in this template made further down:

  • check for data.type_class, instead of data.type
  • check if data.type_class is defined rather than if it's empty

The problem doesn't exist in 2.5.

@hhamon
Copy link
Contributor

hhamon commented Dec 30, 2014

Is it not a duplicate?

@xabbuh
Copy link
Member

xabbuh commented Dec 30, 2014

@hhamon There were previous attempts like #12889 which did more than the changes here (they tried to fix #10054 which is similar but not the same as it seems).

@@ -420,7 +420,7 @@
{% else %}
<div class="toggle-icon empty"></div>
{% endif %}
{{ name|default('(no name)') }} {% if data.type is not empty %}[<abbr title="{{ data.type_class }}">{{ data.type }}</abbr>]{% endif %}
{{ name|default('(no name)') }} {% if data.type_class is defined %}[<abbr title="{{ data.type_class }}">{{ data.type }}</abbr>]{% endif %}
Copy link
Member

Choose a reason for hiding this comment

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

we still need to check for data.type being empty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I will change it in the other place too.

This make the check consistent with a similar check in this template made further down, and fixes an issue with fields added to the form after the form was built.
@jakzal
Copy link
Contributor Author

jakzal commented Dec 30, 2014

This only prevents an exception. It doesn't really fix the root cause as the type won't be shown. I still think it should be merged.

@fabpot
Copy link
Member

fabpot commented Dec 30, 2014

Thank you @jakzal.

@fabpot fabpot merged commit 1e794ca into symfony:2.6 Dec 30, 2014
fabpot added a commit that referenced this pull request Dec 30, 2014
…m after the form was built (jakzal)

This PR was merged into the 2.6 branch.

Discussion
----------

Fix a web profiler form issue with fields added to the form after the form was built

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no, but not because of this PR
| Fixed tickets | #13155
| License       | MIT
| Doc PR        | -

This also make the check consistent with a similar check in this template made further down:
* check for `data.type_class`, instead of `data.type`
* check if `data.type_class` is defined rather than if it's empty

The problem doesn't exist in 2.5.

Commits
-------

1e794ca Check if a field type_class is defined before using it.
@jakzal jakzal deleted the bugfix/form-web-profiler branch December 30, 2014 14:37
nicolas-grekas added a commit that referenced this pull request Dec 12, 2016
…lds added to the form after the form was built (tgalopin)

This PR was merged into the 3.2 branch.

Discussion
----------

[WebProfilerBundle] Fix a web profiler form issue with fields added to the form after the form was built

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

I discovered a bug in 3.2 in the web profiler and this PR fixes it. This issue has been fixed in the past by #13166 and probably reintroduced by the refactoring we did in the WebProfiler in 3.2. I simply applied the fix to these changes.

To reproduce the original problem, simply clone the current standard edition, create a Form with a Type and add a field after its creation. Once done, try to access the Webprofiler: it will fails with the following error:

> Key "type_class" for array with keys "id, name, view_vars, children" does not exist in @WebProfiler/Collector/form.html.twig at line 460.

Commits
-------

d9af8e9 Fix a web profiler form issue with fields added to the form after the form was built
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.

None yet

5 participants