Commit Graph

9151 Commits

Author SHA1 Message Date
Bernhard Schussek 5adec19f56 [Form] Fixed typos 2012-04-11 16:37:42 +02:00
Jeremy Mikola cb87ccb284 [Form] Failing test for empty_data option BC break
This demonstrates the issue described in symfony/symfony#3354. FieldType no longer has access to the child type's data_class option, which makes it unable to create the default closure for empty_data.
2012-04-11 16:37:42 +02:00
Bernhard Schussek b7330456b6 [Form] Fixed option support in Form component 2012-04-11 16:37:42 +02:00
Fabien Potencier 70d49c3c2c merged branch jakzal/FilesystemMirrorCleanup (PR #3844)
Commits
-------

efad5d5 [Filesystem] Prevented infiite loop on windows while calling mirror on symlink. Added test for mirroring symlinks.

Discussion
----------

[Filesystem] Prevented infinite loop on windows while mirrorring symlinks

First check for filetype in *mirror()* method is:

    if (is_link($file)) {
        $this->symlink($file, $target);

later we see:

    } elseif (is_file($file) || ($copyOnWindows && is_link($file))) {
        $this->copy($file, $target, isset($options['override']) ? $options['override'] : false);

The later check for links on windows (*$copyOnWindows && is_link($file)*) won't ever get called. Calling *symlink()* in *mirror()* on windows would lead to calling *mirror()* again.

Note that I didn't actually try running it on windows platform. I added a test for mirroring symlinks (non-windows test). I think it'd be good if someone added some windows specific tests to this class.

I also modified the target path:

    $target = $targetDir.'/'.str_replace($originDir.DIRECTORY_SEPARATOR, '', $file->getPathname());

It didn't use DIRECTORY_SEPARATOR and is equivalent to:

    $target = str_replace($originDir, $targetDir, $file->getPathname());

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: ~
Todo: ~
2012-04-11 16:06:53 +02:00
Fabien Potencier 0c57db1771 merged branch vicb/routing_improvements (PR #3876)
Commits
-------

9307f5b [Routing] Implement bug fixes and enhancements

Discussion
----------

[Routing] Implement bug fixes and enhancements (from @Tobion)

This is mainly #3754 with some minor formatting changes.

Original PR message from @Tobion:

Here a list of what is fixed. Tests pass.

1. The `RouteCollection` states

    > it overrides existing routes with the same name defined in the instance or its children and parents.

    But this is not true for `->addCollection()` but only for `->add()`. addCollection does not remove routes with the same name in its parents (only in its children). I don't think this is on purpose.
    So I fixed it by making sure there can only be one route per name in all connected collections. This way we can also simplify `->get()` and `->remove()` and improve performance since we can stop iterating recursively when we found the first and only route with a given name.
    See `testUniqueRouteWithGivenName` that fails in the old code but works now.

2. There was an bug with `$collection->addPrefix('0');` that didn't apply the starting slash. Fixed and test case added.

3. There is an issue with `->get()` that I also think is not intended. Currently it allows to access a sub-RouteCollection by specifing $name as array integer index. But according to the PHPdoc you should only be allowed to receive a Route instance and not a RouteCollection.
    See `testGet` that has a failing test case. I fixed this behavior.

4. Then I recognized that `->addCollection` depended on the order of applying them. So

        $collection1->addCollection($collection2, '/b');
        $collection2->addCollection($collection3, '/c');
        $rootCollection->addCollection($collection1, '/a');

    had a different pattern result from

        $collection2->addCollection($collection3, '/c');
        $collection1->addCollection($collection2, '/b');
        $rootCollection->addCollection($collection1, '/a');

    Fixed and test case added. See `testPatternDoesNotChangeWhenDefinitionOrderChanges`.

5. PHP could have ended in an infinite loop when one tried to add an existing RouteCollection to the tree. Fixed by throwing an exception when this situation is detected. See tests `testCannotSelfJoinCollection` and `testCannotAddExistingCollectionToTree`.

6. I made `setParent()` private because its not useful outside the class itself. And `remove()` also removes the route from its parents. Added public `getRoot()` method.

7. The `Route` class throwed a PHP warning when trying to set an empty requirement.

8. Fixed issue #3777. See discussion there for more info. I fixed it by removing the over-optimization that was introduced in 91f4097a09 but didn't work properly. One cannot reorder the route definitions, as is was done, because then the wrong route might me matched before the correct one. If one really wanted to do that, it would require to calculate the intersection of two regular expressions to determine if they can be grouped together (a tool that would also be useful to check whether a route is unreachable, see discussion in #3678) We can only safely optimize routes with a static prefix within a RouteCollection, not across multiple RouteCollections with different parents.  This is especially true when using variables and regular expressions requirements.
I could however apply an optimization that was missing yet: Collections with a single route were missing the static prefix optimization with `0 === strpos()`.

9. Fixed an issue where the `PhpMatcherDumper` would not apply the optimization if the root collection to be dumped has a prefix itself. For this I had to rewrite `compileRoutes`. It is also much easier to understand now. Addionally I added many comments and PHPdoc because complex recursive methods like this are still hard to grasp.
I added a test case for this (`url_matcher3.php`).

10. Fix that `Route::compile` needs to recompile a route once it is modified. Otherwise we have a wrong result. Test case added.
2012-04-11 15:53:10 +02:00
Tobias Schultze 9307f5b33b [Routing] Implement bug fixes and enhancements 2012-04-11 15:45:27 +02:00
Fabien Potencier 3469713d90 merged branch kmohrf/ticket_2827_validator_mail_A_RR_DNS_hostcheck (PR #3799)
Commits
-------

f617e02 [Validator] added less-strict email host verification

Discussion
----------

[Validator] added less-strict email host verification

uhhhhh, my first pull request :>. uhm... tell me if i did something wrong :)

#### Request info
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes... well,not really (i guess master branch is not passing - at least i didnt broke the email test)
Fixes the following tickets: #2827

#### Description
New checkHost attribute in email constraint will make the validator check for only one of MX, A or AAAA DNS resource records to verify it as a valid email address.
2012-04-11 14:41:31 +02:00
Fabien Potencier 3d6575e152 merged branch richardmiller/validation_path_upgrade (PR #3874)
Commits
-------

6433ca0 Removed duplicated content
60d9aff Added info on changes to setting validation subpath to UPGRADE-2.1.md

Discussion
----------

Added info on changes to setting validation subpath to UPGRADE-2.1.md

As per GH-3424

---------------------------------------------------------------------------

by stof at 2012-04-11T11:05:00Z

you duplicated it

---------------------------------------------------------------------------

by richardmiller at 2012-04-11T11:07:17Z

I've closed the other one - there was a mess with the commits and an unnecessary merge.

---------------------------------------------------------------------------

by stof at 2012-04-11T11:30:00Z

@richardmiller I'm talking about the content

---------------------------------------------------------------------------

by richardmiller at 2012-04-11T11:35:40Z

@stof, ah sorry misunderstood, one of those days it seems :)

---------------------------------------------------------------------------

by stof at 2012-04-11T11:42:21Z

I should have commented inline to make it more clear
2012-04-11 14:39:22 +02:00
Richard Miller 6433ca0936 Removed duplicated content 2012-04-11 12:34:00 +01:00
Richard Miller 60d9aff29a Added info on changes to setting validation subpath to UPGRADE-2.1.md 2012-04-11 11:44:45 +01:00
Fabien Potencier 3c3ec5c677 merged branch tvlooy/GetSetMethodNormalizer (PR #3582)
Commits
-------

039ff6f allow more control on GetSetMethodNormalizer by using callback functions and an ignoreAttributes list

Discussion
----------

allow more control on GetSetMethodNormalizer

Here is an other attempt. You would use this as follows:

    $serializer = new \Symfony\Component\Serializer\Serializer(
        array(new \Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer()),
        array('json' => new \Symfony\Component\Serializer\Encoder\JsonEncoder())
    );

    $callbacks = array('books' => function ($books) { return NULL; });

    return new Response(
        $serializer->serialize($paginator->getRows(), 'json', $callbacks),
        200,
        array('Content-Type' => 'application/json')
    );

Besides of returning NULL, you could also do things like:

    $callbacks = array(
        'books' => function ($books) {
            $ids = array();
            foreach ($books as $book) {
                $ids[] = $book->getId();
            }
            return $ids;
        },
        'author' => function ($author) {
            return $author->getId();
        },
        'creationDate' => function ($creationDate) {
            return $creationDate->format('d/m/Y');
        },
    );

The commit is not complete yet. But at this point I am interested in your opinions.

---------------------------------------------------------------------------

by lsmith77 at 2012-03-12T22:53:18Z

in general i agree that using a callback is a good solution to provide more power without complicating the API or implementation in this case.

please add a test case, this should also help illustrate how this can be used in practice.

---------------------------------------------------------------------------

by schmittjoh at 2012-03-13T04:54:33Z

Note that your change breaks the API defined by the interface, i.e. someone using this method needs to type-hint the serializer implementation, not the interface.

It also adds a parameter to the public API of the serializer which will only work with one specific normalizer. What if another normalizer needs additional information, should another parameter be added to the serialize method? What about deserialization?

Bottom line is, the serializer component was simply not designed for this kind of thing. I've tried to make it more flexible before creating the bundle, but some things simply cannot be fixed in a sane way.

---------------------------------------------------------------------------

by tvlooy at 2012-03-13T06:07:45Z

Would just adding a setCallbacks() to the GetSetMethodNormalizer be a better solution? That doesn't touch the API. I will try to write some tests this evening.

---------------------------------------------------------------------------

by schmittjoh at 2012-03-13T16:22:50Z

That would definitely be better.

You would then need to retrieve the normalizer instance before calling ``serialize`` on the serializer which also leaves a stale taste, but I have no other solution for now.

---------------------------------------------------------------------------

by tvlooy at 2012-03-13T21:32:26Z

So, this should be it then. Yet an other usage example:

    $normalizer = new \Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer();
    $normalizer->setCallbacks(
        array(
            'books' => function ($books) {
                $ids = array();
                foreach ($books as $book) {
                    $ids[] = $book->getId();
                }
                return $ids;
            },
        )
    );
    $serializer = new \Symfony\Component\Serializer\Serializer(
        array($normalizer),
        array('json' => new \Symfony\Component\Serializer\Encoder\JsonEncoder())
    );

    return new Response(
        $serializer->serialize($paginator->getRows(), 'json'),
        200,
        array('Content-Type' => 'application/json')
    );

---------------------------------------------------------------------------

by tvlooy at 2012-03-18T21:16:48Z

Anything else needed for this to get pulled in?

---------------------------------------------------------------------------

by tvlooy at 2012-03-19T18:33:58Z

Hm, I like to keep it that way because I like the fact that not passing a callable will result in a warning instead of silently skipping it. You don't get that behaviour by treating it as null.

---------------------------------------------------------------------------

by vicb at 2012-03-19T23:15:37Z

I was unclear: the code should throw an exception when an element is not callable, this is why `null` will not be supported any more (it is not a callback as the `setCallbacks` indicate).

They are several way to support the former behavior:

* the cb can return a defined interface,
* the cb can throw a defines exc,
* by adding a `setIgnoredAttributes` method

Please also squash your commits.

---------------------------------------------------------------------------

by tvlooy at 2012-03-20T21:02:06Z

Yes, I like the setIgnoredAttributes solution. I changed it and squashed the commits.

---------------------------------------------------------------------------

by tvlooy at 2012-03-26T20:07:36Z

some improvements and squashed the commits

---------------------------------------------------------------------------

by stof at 2012-04-03T22:36:15Z

@tvlooy Please rebase your branch. It conflicts with master because of the move of tests.

---------------------------------------------------------------------------

by tvlooy at 2012-04-04T07:43:47Z

@stof I will do it on saturday, if that is ok with you.

---------------------------------------------------------------------------

by fabpot at 2012-04-10T18:29:30Z

Is it mergeable now? ping @Seldaek, @schmittjoh.

---------------------------------------------------------------------------

by tvlooy at 2012-04-10T18:55:04Z

yes, it should be
2012-04-11 12:08:10 +02:00
Fabien Potencier ff9b7316d1 updated CHANGELOG 2012-04-11 11:51:04 +02:00
Fabien Potencier 22d8a436fa merged branch kimhemsoe/xcache_classloader (PR #3809)
Commits
-------

c36651b Fixed spelling error
f123684 Removed leftover from c/p
b74a5d4 Updated to new cache loader pattern.
7e66908 Added XCache class loader

Discussion
----------

[ClassLoader] Added XCache class loader

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

There is no tests, as it seems there is no way to use xcache storage functions from CLI.

---------------------------------------------------------------------------

by stof at 2012-04-06T20:12:09Z

Please implement a XcacheClassLoader following the same pattern than the new ApcClassLoader instead

---------------------------------------------------------------------------

by cordoval at 2012-04-07T14:20:47Z

- should include tests
- should include documentation (will you also update the component documentation for this new class?)

---------------------------------------------------------------------------

by stof at 2012-04-07T14:25:00Z

@cordoval the PR explains why there is no tests: xcache canot be used in the CLI.

---------------------------------------------------------------------------

by cordoval at 2012-04-07T14:26:43Z

ok @stof sorry it said it seemed not to be possible, i thought it was possible but I am wrong.

---------------------------------------------------------------------------

by kimhemsoe at 2012-04-07T15:01:24Z

@cordoval My english is really horrible. I would not mind if someone else could do that task for me. We also need to add doc for the new ApcClassLoader.

---------------------------------------------------------------------------

by cordoval at 2012-04-07T15:03:57Z

I wish you can explain me more then about this class and how to use it in code so then I can write easily the documentation :D deal?

---------------------------------------------------------------------------

by kimhemsoe at 2012-04-07T15:21:25Z

Deal :P

The XcacheClassLoader and ApcClassLoader replaces the old ApcUniversalClassLoader.
They giving us support for using another loader then UniversalClassLoader, without duplicating the cache layer.
Aslong it have a public function findFile($class) method.

 $loader = new ClassLoader();

// register classes with namespaces
$loader->add('Symfony\Component', __DIR__.'/component');
$loader->add('Symfony', __DIR__.'/framework');

$cachedLoader = new XcacheClassLoader('my_prefix', $loader);

// activate the cached autoloader
$cachedLoader->register();

Think that is more or less the essence of this.

---------------------------------------------------------------------------

by cordoval at 2012-04-09T08:28:53Z

it is not add but registerNamespace right?

so the main idea is to get rid of the restriction to use Apc with Universal loader

what is the comparative advantage between APC and Xcache?

---------------------------------------------------------------------------

by kimhemsoe at 2012-04-09T08:55:23Z

Yes if the $loader (class finder) were to be a instance UniversalClassLoader.

Yes the main idea is to be able to reuse the cache layer with any class loader there obey to the one restriction.

Difference between apc and xcache and why to use what is coming down to taste and your setup. We use xcache as APC have some issues in fastcgi setups. when we upgrade to php54 at somepoint we get to chance to move to php-fpm wich solves these issues. Short story: Slightly out of scope for any documentation in here :-P
2012-04-11 11:50:49 +02:00
Fabien Potencier 191aaa3f0c merged branch bschussek/issue3635 (PR #3820)
Commits
-------

65aa387 [Form] Fixed index generation in EntityChoiceList if ID is not an integer

Discussion
----------

[Form] Fixed index generation in EntityChoiceList if ID is not an integer

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3635
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3635)
2012-04-11 11:36:22 +02:00
Fabien Potencier 5e81389b3f merged branch Tobion/patch-1 (PR #3870)
Commits
-------

810b2e1 fix formatting of changelog-2.1.md

Discussion
----------

fix formatting of changelog-2.1.md

Fix indention for correct display. Also a few language fixes.
2012-04-11 09:39:11 +02:00
Fabien Potencier e9539d8a6d merged branch hhamon/command_description_fixes (PR #3871)
Commits
-------

b4f0a04 [TwigBundle] fixed twig:lint command description.
809933f [FrameworkBundle] fixed translation:update command description.

Discussion
----------

Command description fixes

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
2012-04-11 09:36:53 +02:00
Fabien Potencier c7ba1b9605 merged 2.0 2012-04-11 09:33:57 +02:00
Fabien Potencier 9d5743b35c [Console] fixed typo 2012-04-11 09:12:51 +02:00
Hugo Hamon b4f0a04574 [TwigBundle] fixed twig:lint command description. 2012-04-11 08:58:25 +02:00
Hugo Hamon 809933f55b [FrameworkBundle] fixed translation:update command description. 2012-04-11 08:57:57 +02:00
Tobias Schultze 810b2e134c fix formatting of changelog-2.1.md 2012-04-11 09:34:40 +03:00
Fabien Potencier 88353575e4 merged branch vicb/routing_dumpers (PR #3858)
Commits
-------

77185e0 [Routing] Allow spaces in the script name for the apache dumper
6465a69 [Routing] Fixes to handle spaces in route pattern

Discussion
----------

[Routing] Handling of space characters in the dumpers

The compiler was using the 'x' modifier in order to ignore extra spaces and line feeds but the code was flawed:

- it was actually ignoring all the spaces, not only the extra ones added by the compiler,
- all the spaces were stripped in the php and apache matchers.

The proposed fix:

- do not use the 'x' modifier any more (and then do no add extra spaces / line feeds),
- do not strip the spaces in the matchers,
- escapes the spaces (both in regexs and script name) for the apache matcher.

It also include [a small optimization](https://github.com/vicb/symfony/pull/new#L9L89) when the only token of a route is an optional variable token - the idea is to make the regex easier to read.

---------------------------------------------------------------------------

by vicb at 2012-04-10T13:59:45Z

@Baachi fixed now. Thanks.

---------------------------------------------------------------------------

by Tobion at 2012-04-10T16:01:31Z

+1, I saw no reason for pretty printing the regex in the first place (just for debugging I guess).
@vicb since you want to make the regex easier to read, I propose the remove the `P` from the variable regex `?P<bar>`, which is not needed anymore in PHP 5.3 (and we only support PHP 5.3+ anyway).

---------------------------------------------------------------------------

by vicb at 2012-04-10T16:08:36Z

@Tobion could you make a PR to this branch for the named parameters ?

---------------------------------------------------------------------------

by Tobion at 2012-04-10T16:12:34Z

I can include it in #3754 because I'm about the add 2 more fixes to it anyway.
But when I proposed to apply these fixes to 2.0 Fabien rejected it. So not sure what branch you want me to apply this.

---------------------------------------------------------------------------

by vicb at 2012-04-10T16:25:38Z

May be the best is to put it on hold while I am reviewing your PRs. There are already enough changes, we'll make an other PR after all have been sorted out.

What's the difference between 3754 and 3810 ? (3810 + 3763 = 3754 ?)

---------------------------------------------------------------------------

by Tobion at 2012-04-10T16:39:32Z

Lol you forget to link the PR numbers. At first sight I thought it's some sort of mathematical riddle. Haha
#3810 is for 2.0 =  #3763 (already merged) + #3754 for master

---------------------------------------------------------------------------

by vicb at 2012-04-10T16:52:18Z

I didn't link on purpose... the question is if '=' means strictly or loosely equal (any diffs - beside master vs 2.0) ?

---------------------------------------------------------------------------

by Tobion at 2012-04-10T17:06:04Z

It just applies my changes to 2.0. Nothing more. So master still differs from 2.0 by the addional features that were already implemented (e.g. `RouteCollection->addCollection` with optional requirements and options). But since my changes are bug fixes (except the performance improvement in #3763 but that doesn't break anything and makes 2.0 easier to maintain) I thought they should go into 2.0 as well.

---------------------------------------------------------------------------

by vicb at 2012-04-10T17:14:27Z

@Tobion only bug fixes mean "only bug fixes". You should re-open a PR for 2.0 with "only bug fixes", you might want to wait for me to review 3754.

---------------------------------------------------------------------------

by Tobion at 2012-04-10T17:21:00Z

Without #3763 it's much harder to apply the bug fixes. And now that I found 2 more bugs which requiresome rewriting of the PhpMatcherDumper, I don't want to apply all the commits by hand again for 2.0...
2012-04-11 08:28:45 +02:00
Fabien Potencier e21d4ffde4 [Console] fixed CS 2012-04-11 08:26:14 +02:00
Fabien Potencier abe8ba1e1c merged branch jfsimon/master (PR #3869)
Commits
-------

5b5b2c8 [Console] Fixed and added formatter tests.
4ee8cfb [Console] Updated formatter to use style stack.
bd1d28c [Console] Added formatter style stack tests.
b63bd0e [Console] Added formatter style stack.

Discussion
----------

[Console] Fixed formatter nested style appliance in a proper way.

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes

When outputing styled text in the console, you sometimes face to a confusing behavior: style tags cannot be nested. If tou try something like `<fg=blue>Hello <fg=red>world</fg=red>!</fg=blue>`, the trailing `!` will not be styled.

This PR introduce a new FormatterOutputStyleStack to keep open/closed styles informations up-to-date.
2012-04-11 08:25:23 +02:00
Jean-François Simon 5b5b2c81c4 [Console] Fixed and added formatter tests. 2012-04-11 08:04:59 +02:00
Jean-François Simon 4ee8cfb81e [Console] Updated formatter to use style stack. 2012-04-11 07:58:51 +02:00
Jean-François Simon bd1d28cb50 [Console] Added formatter style stack tests. 2012-04-11 07:58:28 +02:00
Jean-François Simon b63bd0e7e0 [Console] Added formatter style stack. 2012-04-11 07:58:13 +02:00
Fabien Potencier 02e1b81f65 merged 2.0 2012-04-10 20:26:56 +02:00
Fabien Potencier 57990cc53f merged branch johannes85/2.0 (PR #3791)
Commits
-------

0024ddc Fix for using route name as check_path.

Discussion
----------

Security Bundle route as check_path

In the current 2.0 branch you can't use a route as
firewalls:
admin_area:
login_path:
you will get a InvalidConfigurationException.

In the 2.1 version this is fixed. Since 2.1 isn't released i think this fix should be merged into the 2.0 branch too. Many people have this problem (https://github.com/schmittjoh/JMSI18nRoutingBundle/issues/7) for example which effectively blocks internationalisation in combination with the firewall.

---------------------------------------------------------------------------

by stof at 2012-04-10T13:35:13Z

@fabpot ping
2012-04-10 20:26:31 +02:00
Fabien Potencier c7b226442b merged branch bschussek/issue3732 (PR #3819)
Commits
-------

c4e68a3 [Form] Moved logic of addXxx()/removeXxx() methods to the PropertyPath class

Discussion
----------

[Form] Moved logic of addXxx()/removeXxx() methods to the PropertyPath class

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3732
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3732)

The addXxx()/removeXxx() methods should now be called correctly in ChoiceType and CollectionType.

PropertyPath now favors addXxx()/removeXxx() over setXxx() for collections. For example:

```
$propertyPath = new PropertyPath('article.tags');

// Tries to use addTag()/removeTag() and only uses setTags() (et al.)
// if not found
$propertyPath->setValue($article, $tags);
```

For other languages than English or very irregular plurals, a custom singular can be set by separating it with a pipe:

```
$propertyPath = new PropertyPath('article.genera|genus');
```

---------------------------------------------------------------------------

by bschussek at 2012-04-07T12:40:39Z

Again, the failing build is not my fault.
2012-04-10 20:22:54 +02:00
Fabien Potencier 517f8e0162 merged branch bschussek/issue3839 (PR #3859)
Commits
-------

61d792e [Form] Changed checkboxes in an expanded multiple-choice field to not include the choice index
bc9bc4a [Form] Fixed behavior of expanded multiple-choice field when submitted without ticks
2e07256 [Form] Simplified choice list API
2645120 [Form] Fixed handling of expanded choice lists, checkboxes and radio buttons with empty values ("")

Discussion
----------

[Form] Fixed handling of empty values in checkbox/radio/choice type

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3839, #3366
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3839)

This PR fixes the processing of checkboxes and radio buttons with empty "value" attributes as well as of expanded choice forms with empty values. Additionally, some unnecessary complexity has been removed of the new ChoiceList API.

---------------------------------------------------------------------------

by stof at 2012-04-10T13:56:12Z

You probably need to change some things in the CHANGELOG file too

---------------------------------------------------------------------------

by bschussek at 2012-04-10T14:39:24Z

No. This is an update of a previous post-2.0 PR, the CHANGELOG is still accurate.

---------------------------------------------------------------------------

by stof at 2012-04-10T14:46:47Z

well, doesn't it require changes to the description of previous changes related to the ChoiceList ? It does in the UPGRADE file so it is weird if the CHANGELOG does not require any change.

---------------------------------------------------------------------------

by bschussek at 2012-04-10T14:56:05Z

Feel free to check yourself :)
2012-04-10 20:02:12 +02:00
Fabien Potencier 48abdaff3c merged branch bschussek/issue3278 (PR #3860)
Commits
-------

004c873 [Form] Fixed display of DateTimeType and TimeType when displayed as "single_text" and "with_seconds" is false

Discussion
----------

[Form] Fixed display of [Date]TimeType when displayed as "single_text" and "with_seconds" is false

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3278
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3278)
2012-04-10 19:55:43 +02:00
Fabien Potencier 837960f3c4 merged branch yethee/sync_ru_trans (PR #3863)
Commits
-------

362ac78 [FrameworkBundle] Sync Russian translations

Discussion
----------

[FrameworkBundle] Sync Russian translations
2012-04-10 19:54:15 +02:00
Deni 362ac78463 [FrameworkBundle] Sync Russian translations 2012-04-10 21:33:35 +04:00
Bernhard Schussek 004c873c74 [Form] Fixed display of DateTimeType and TimeType when displayed as "single_text" and "with_seconds" is false 2012-04-10 18:15:31 +02:00
Bernhard Schussek 61d792eebd [Form] Changed checkboxes in an expanded multiple-choice field to not include the choice index 2012-04-10 17:47:40 +02:00
Bernhard Schussek bc9bc4af5d [Form] Fixed behavior of expanded multiple-choice field when submitted without ticks 2012-04-10 17:42:05 +02:00
Victor Berchet 77185e0998 [Routing] Allow spaces in the script name for the apache dumper 2012-04-10 17:29:43 +02:00
Victor Berchet 6465a6987a [Routing] Fixes to handle spaces in route pattern
- The route compiler does not add extra space or line-feed,
- The generated regex does not use the 'x' modified any more,
- The PHP and apache matchers do not need to strip any chars (vs space and line feed before),
- The space characters are escaped according to the apache format
2012-04-10 17:29:34 +02:00
Bernhard Schussek 2e07256921 [Form] Simplified choice list API 2012-04-10 15:37:24 +02:00
Bernhard Schussek 26451201e0 [Form] Fixed handling of expanded choice lists, checkboxes and radio buttons with empty values ("") 2012-04-10 15:35:46 +02:00
Fabien Potencier 07e261ea9e removed obsolete files 2012-04-10 11:50:32 +02:00
Fabien Potencier 667f8fd474 merged branch brikou/typehint_fix (PR #3853)
Commits
-------

6486e25 fixed used statements
3530858 fixed typehints in docblocks (IDE completion works now)

Discussion
----------

Typehint fix

---------------------------------------------------------------------------

by fabpot at 2012-04-10T09:12:51Z

For type hint in PHPDoc, we are using the class name and not the full namespace (of course, we need the associated use statement).

---------------------------------------------------------------------------

by brikou at 2012-04-10T09:29:39Z

@fabpot ok fabien, i've added the use statements and (for both entityRepository and manager) cheer
2012-04-10 11:43:16 +02:00
Brikou CARRE 6486e25a38 fixed used statements 2012-04-10 11:28:30 +02:00
Fabien Potencier a8cfa4a7de merged branch pminnieur/patch-2 (PR #3852)
Commits
-------

a56bea6 removed `param_converter` from symfony-1.0.xsd

Discussion
----------

removed `param_converter` from symfony-1.0.xsd

I think its a relict of the SensioFrameworkExtraBundle?
2012-04-10 11:13:14 +02:00
Brikou CARRE 353085857b fixed typehints in docblocks (IDE completion works now) 2012-04-10 11:04:07 +02:00
Pierre Minnieur a56bea6d88 removed `param_converter` from symfony-1.0.xsd 2012-04-10 12:00:57 +03:00
Fabien Potencier 8860424823 merged branch vicb/url_decoding (PR #3780)
Commits
-------

55014a6 [Routing] Request methods always return a raw path, fix the matcher to decode only once
d17ba0e Fixed base URL detection when request URI contains encoded chars

Discussion
----------

[RFC] Fix issues with url decoding

Related: #2324, #2963, #2962, #2579

### This PR fixes two issues:

* `+` in paths were turned to " " by `urldecode()`
* `urldecode()` was called a few times (and a different number of times according to which part of the path was handled, see #2962 for details).

### BC Breaks:

* `Request::getPathInfo()`, `Request::getBaseUrl()` and `Request::getBasePath` now return the raw (encoded) path (vs a decoded path before this PR). You should check any calls to these methods in your code and wrap them in `rawurldecode()` if needed.
* The `UrlMatcher` now decodes the URL only once (i.e. variable are no more decoded twice) and use `rawurldecode()` to support `+`.

### Notes:

* @arnaud-lb, the first commit is based on your #2963 so I put your name for the commit,

### Comment history from the original PR:

@vicb

**The state before this PR:**

* getRequestUri() returns an **encoded** value
* getPathInfo() returns a **decoded** value
* getBaseUrl() returns a **decoded** value
* getBasePath() returns a **decoded** value

The decoded value is wrong as `urldecode` is used in place of `rawurldecode` turning `+` into a space character (#2324).

The matcher starts by urldecoing the path (it is already decoded as explained right before) and then urldecodes each variable one more time.

We end up with a path being decoded twice and variables being decoded three times.

`Request::getUri()` calls both `getBaseUrl()` and `getPathInfo()` so that the return URI is **decoded**.

**The state after the PR:**

* getRequestUri() returns an **encoded** value
* getPathInfo() returns an **encoded** value
* getBaseUrl() returns an **encoded** value
* getBasePath() returns an **encoded** value

We are consistent and we have the raw values everywhere - there is no (easy) way to get the encoded value back once it has been decoded as it is done in the current code.

The matcher relies on an encoded value and decode the value only once (using `rawurldecode` to support `+`s).

So basically this PR:

* fix a bug - URL with `+` are now supported,
* makes paths consistent - encoded values everywhere, including `getUri()`
* makes variables consistent: they are decoded only once - the same as query string parameters.

There are some BC breaks:

* getPathInfo() returns an encoded value vs a decoded one before,
* getBaseUrl() returns an encoded value vs a decoded one before.
* getBasePath() returns an encoded value vs a decoded one before.

Any code relying on the output of one of the 2 previous methods should be checked and upgraded if needed. I am interested in the use cases where your code need to be updated.

@Seldaek

I checked a few projects and this is what I found (usage of getPathInfo & getBaseUrl):

- One use case of getPathInfo to check if the url started with `/something/` which is a prefix used for all "overlay" content which had to be treated differently somewhere => most likely unaffected
- One use case for checking path prefixes by regex in our [CorsBundle](https://github.com/nelmio/NelmioCorsBundle/blob/master/EventListener/CorsListener.php#L52-56) => potentially affected depending on the complexity of regexes I'd say

@vicb

Thanks @Seldaek for reporting the use cases. You second case would be solved by `rawurldecode`ing the path info which is a minimal change.

And in general I think we have to expand to doc to specify the url format that should be used.

---------------------------------------------------------------------------

by vicb at 2012-04-04T13:42:21Z

I'll squash the commits before this gets merged but for now it make the review easier.
2012-04-10 10:43:27 +02:00
Victor Berchet 55014a6841 [Routing] Request methods always return a raw path, fix the matcher to decode only once
sq
2012-04-10 10:40:58 +02:00