Commits
-------
2c1108c [Form] Revert the ability to override anything else than the text of the label while rendering a row
da467a6 [Form] Fix the exception message when no block is found while rendering
8670995 [Form] Optimize rendering when the block to render is known
41e07c9 [Form] Optimize rendering
ee5d975 [Form] Remove a test which is no more relevant (after recent FileType refactoring)
f729c6b [Form] Add the ability to override label & widget options when rendering a row
e09ae3f [Form][FrameworkBundle] Make FormHelper::renderSection() recursively callable, introduce FormHelper::renderBlock()
e43fb98 [Form][TwigBridge] Make FormExtension::render() recursively callable to ease theming
Discussion
----------
[Form] Some refactoring of the rendering
# First two commits
## FormExtension::render() can now be called recursively.
The main use case is theming support in for collections. Let's consider that you have a collection of `CustomType`, the type hierarchy while rendering the proto would be `field < form < custom < prototype`. Before this change any theme applied to your custom type (i.e. a `custom_row` block) would not have been taken into account while rendering the prototype because of the structure of the `prototype_row` block:
```html
{% block prototype_row %}
{% spaceless %}
<script type="text/html" id="{{ proto_id }}">{{ block('field_row') }}</script>
{% endspaceless %}
{% endblock prototype_row %}
```
which skip the `custom_row` block rendering to fallback to the `field_row` block rendering.
With this PR `prototype_row` recursively calls `FormExtension::render()`
```html
{% block prototype_row %}
{% spaceless %}
<script type="text/html" id="{{ proto_id }}">{{ form_row(form) }}</script>
{% endspaceless %}
{% endblock prototype_row %}
```
this has for effect to render the block for the parent type (i.e. `custom_row`)
## FormHelper
The `FormHelper` has been updated to more closely match the `FormExtension` architecture and the templates have been modified accordingly. `echo $view['form']->renderBlock(<block name>)` is the php equivalent of `{{ block(<block name>) }}`.
The attributes are now rendered using a template rather than by the `FormHelper::attributes()` method.
Several templates have been fixed.
# Third commit
The `$varStack` property was used to forward options to the label and the widget when rendering a row. The implementation was not working as expected. The proposed way to override label and widget options is to pass these options in the `label` and `widget` keys while callinf `render_row`.
That would be:
`{{ form_row(form.field, {"attr": {<row attributes>}, "label" : {"label": <text>, "attr": {<label attr>}}, "widget" : { "attr" : {<widget attributes}} } }}`
So there is now the ability to set attributes for the row (`<div>` or `<tr>`).
This has been discussed on [the mailing list](http://groups.google.com/group/symfony-devs/browse_thread/thread/17754128ba480545). **I would like to find a compromise with @Seldaek before this gets merged**
The `$varStack` property is now only used when recursively calling `FormExtension::render()`
# Notes
I have preferred to submit several commits in order to ease review and to keep some history.
---------------------------------------------------------------------------
by stof at 2011/06/20 05:20:56 -0700
@vicb On a side note, do you think it would be possible to support form theming in PHP templates too ? Currently, the only way to customize the rendering of forms when using PHP templates is to overwrite the FrameworkBundle's templates, and this impacts all forms. This makes the PHP rendering far less powerful than the Twig one.
I don't know the Form rendering and the PHPEngine well enough to know if it is feasible for 2.1 or not.
---------------------------------------------------------------------------
by vicb at 2011/06/20 05:35:11 -0700
@stof I hope to make it possible but I need a little bit more thinking to find the best possible solution which should not look like a hack.
---------------------------------------------------------------------------
by vicb at 2011/06/21 01:13:10 -0700
This should not be merged yet, it might have some issue with the variable stack. I am working on it.
---------------------------------------------------------------------------
by vicb at 2011/06/21 01:41:11 -0700
Sorted out the issue, it was linked to some local _optimization_, the code of this PR is ok.
---------------------------------------------------------------------------
by vicb at 2011/06/21 02:01:24 -0700
I have pushed a [POC of php theming based on this PR](https://github.com/vicb/symfony/commits/form%2Fphp-theme) to my repo - it is lacking a configuration and cache layer.
I have open [a thread on the ml](http://groups.google.com/group/symfony-devs/browse_thread/thread/9b3f131fe116b511) to discuss this.
---------------------------------------------------------------------------
by vicb at 2011/06/21 23:40:21 -0700
@fabpot fixed in the last commit.
Commits
-------
2d13129 Added an autoloader for doctrine proxies
Discussion
----------
Added an autoloader for doctrine proxies
This registers an autoloader for Doctrine proxies to avoid issues when unserializing proxies. See symfony/symfony-standard#91 for the original discussion.
Not using the CLassLoader component is intended as nothing enforces the proxy directory to follow the PSR-0 convention for the proxy namespace (the default value does but it is not mandatory)
Commits
-------
1cb5164 Replace some double quotes to single quotes
12259d1 Replace optionNal to optional
Discussion
----------
Fixed typo and double quotes within some strings
Fixed typo and double quotes within some strings
Commits
-------
2f78a44 Changed recognition of constraints in annoations
Discussion
----------
[Validator] Changed recognition of constraints in annotations
I accidently added a constraint to a setter method of an entity:
/**
* @Assert\MinLength(2)
*/
public function setName($name)
{ ... }
The result was that the framework generated the following exception:
Neither method getTName nor isTName exists in class Blogsh\TestBundle\Entity\User
The problem is that ClassMetadata::addGetterConstraint is called even if the inspected method isn't a getter or an isser.The fix changes the behaviour so that an exception is thrown, saying that it is not valid to add a constraint there. I'm not sure whether this is the right way to handle that problem. It would also be possible to remove the exception and just ignore constraints added to inapplicable methods.
However, the fix prevents the strange error messages from the user.
Commits
-------
159fc0e [Validator] Added symbols to IDNs validation
c827faf [Validator] Add support for IDNs and custom TLDs
Discussion
----------
[Validator] Add support for IDNs and custom TLDs
Minor changes to allow for IDNs and [custom TLDs](http://news.softpedia.com/news/ICANN-Approves-New-Custom-Generic-Top-Level-Domains-Like-google-bank-206977.shtml). This is the only sane way to support everything in a timeless manner.
---------------------------------------------------------------------------
by stealth35 at 2011/06/20 04:32:09 -0700
maybe it should be check the host with idn_to_ascii (if function exists, maybe it's should recreate un punycode en/decoder in the stub)
---------------------------------------------------------------------------
by mvrhov at 2011/06/20 04:40:10 -0700
/me :faceslap.
Haven't seen the link in PR
---------------------------------------------------------------------------
by Seldaek at 2011/06/20 04:40:40 -0700
@mvrhov: Yup, that's what pushed me to reconsider adding this.
@stealth35: I'm not sure if this is needed. I don't want this to be too strict, with another validator or with an extra option I think we can make a check that the domain actually exists, or do a GET / on it or something, but this just checks validity of the syntax.
---------------------------------------------------------------------------
by stealth35 at 2011/06/20 04:48:05 -0700
I understand :)
what about funny IDN like : [☎.com] (http://xn--y3h.com/) ?
---------------------------------------------------------------------------
by Seldaek at 2011/06/20 04:53:19 -0700
@stealth35: Fixed
---------------------------------------------------------------------------
by stealth35 at 2011/06/20 04:56:18 -0700
it's seem great,for acceptable chars [RFC] (http://www.faqs.org/rfcs/rfc3490.html) said (with UseSTD3ASCIIRules option) :
(a) Verify the absence of non-LDH ASCII code points; that is, the
absence of 0..2C, 2E..2F, 3A..40, 5B..60, and 7B..7F.
Commits
-------
58b6403 Adjusted UPDATE
7350109 Renamed core.* events to kernel.* and CoreEvents to KernelEvents
edbdf7b Rename kernel.listener to kernel.event_listener
Discussion
----------
Rename kernel.listener to kernel.event_listener
Better consistency with doctrine.event_listener
---------------------------------------------------------------------------
by schmittjoh at 2011/06/20 07:39:18 -0700
I think "symfony.event_listener" would be even better then, or just short "event_listener".
There are some more inconsistencies:
- "core.response" -> "kernel.response" / "http_kernel.response"
- "core.request" -> "kernel.request" / "http_kernel.request"
- "core.controller" -> "kernel.controller" / "http_kernel.controller"
- "core.view" -> "kernel.view" / "http_kernel.view"
---------------------------------------------------------------------------
by Seldaek at 2011/06/20 07:45:53 -0700
I proposed `event_listener` but @fabpot didn't like it, `symfony.event_listener` could do, but usual we refer to global stuff as framework more than symfony, so `framework.event_listener`?
The point is that the kernel "owns" the event dispatcher, even if anyone can dispatch his stuff through it. You still listen on the kernel dispatcher. So it's not that wrong imo.
As for the others. I agree that the "core" name is a bit strange. kernel would be more suitable imo. @fabpot?
---------------------------------------------------------------------------
by stof at 2011/06/20 07:50:35 -0700
@Seldaek the issue with having ``kernel`` in the tag name is that some user think that they need to define their own tag when they want to dispatch their own events (I faced it a few times on #symfony and it was also the case in #1298)
---------------------------------------------------------------------------
by Seldaek at 2011/06/20 07:51:55 -0700
Well yeah that's why I liked the idea of having only `event_listener`. IMO even with `framework.event_listener` the confusion can still happen.
---------------------------------------------------------------------------
by fabpot at 2011/06/20 08:00:23 -0700
Renaming `core.response` to `kernel.response` is fine with me.
---------------------------------------------------------------------------
by Seldaek at 2011/06/21 07:53:13 -0700
Ok, added another commit that does that renaming. Updated the two related PRs in -docs and -standard as well.
Commits
-------
e272d56 [WebProfilerBundle] Fixes toolbar content check
Discussion
----------
[WebProfilerBundle] Fixes toolbar content check
It appears that some html optimizers trim the comments, therefore the old check was not working. This is more robust.
Fixes the issue reported in 8541a5bcbc
Commits
-------
abd60ac [WebProfilerBundle] Do not display toolbar loading result if it's not a valid toolbar
406c8d8 [WebProfilerBundle] Make toolbar loading non-blocking
Discussion
----------
Non-blocking WDT & prevents garbage to slip in the page
I made the loading non-blocking so that it's not preventing normal operation of the page when the WDT takes a bit long to come up (happens sometimes when the machine is busy).
The second commit also checks that the response looks correct, to prevent stack traces and such to appear in the page if there was a problem. The main issue is not really stack traces though it's mostly with security and intercept_redirect enabled, if you look at a fully secured site you get twice the redirect intercept message to the login page.
Tested in IE7/9/FF4/Opera11
Commits
-------
f315ad9 [WebProfilerBundle] Make sure the toolbar closes properly
Discussion
----------
[WebProfilerBundle] Make sure the toolbar closes properly
Due to the whitespace element between the div which clears and the toolbar div, in some browsers it was left over after you close the toolbar, this doesn't happen anymore.
Tested in IE7/9/FF4/Opera11
Commits
-------
6c46a3b [FrameworkBundle] Prevent breakage when an array callback is not callable
Discussion
----------
[FrameworkBundle] Prevent breakage when an array callback is not callable
W/o this you get warnings that objects can't be converted to strings.
Commits
-------
f400124 [HttpFoundation] Small optimization of Session flash handling
Discussion
----------
[HttpFoundation] Small optimization of Session flash handling
Due to copy-on-write, this is faster and takes less memory unless the flashes are changed during the request, but that's not very likely as typically you set flashes, redirect, then show them, and at that point you do not modify them again.
