merged branch egeloen/password-type (PR symfony#6007)

fabpot · fabpot · commit 68ae2070024f · 2012-11-16T15:57:52.000+01:00
This PR was merged into the master branch. Commits ------- 97f6a1b [Form] Update password type trimming to false Discussion ---------- [Form] Update password trimming to false by default Bug fix: yes Feature addition: no Backwards compatibility break: yes Symfony2 tests pass: yes Fixes the following tickets: ~ Todo: - License of the code: MIT Documentation PR: ~ Hey! Today, I realize that the password type is by default trimmed. IMHO, this is not the expected behavior. By default, the password type should not trim the input value. Regards --------------------------------------------------------------------------- by nomack84 at 2012-11-13T19:16:29Z :+1: --------------------------------------------------------------------------- by mvrhov at 2012-11-13T19:57:29Z IMHO password and username fields should be trimmed. whitespace at the beginning and at the end of those fields are not wanted. At least I don't want to deal with a user support where WS on those fields is not trimmed. --------------------------------------------------------------------------- by egeloen at 2012-11-13T20:08:08Z @mvrhov I agree with you about username fields and other "text" fields but in case of a password field, if the end user specifies white space at the begin/end of his password, it should not be trimmed. It should simply let it as it is. I open this PR due to two customers who reports me this behavior. --------------------------------------------------------------------------- by clemherreman at 2012-11-14T10:06:15Z @mvrhov I agree, username shouldn't be trimmed, however password are kind of special. They should be used *"as is"*, as lots of users have wicked passwords. Moreover, usually the password is asked twice, so if there are spaces, they are most likely wanted by the end user. So 👍 --------------------------------------------------------------------------- by clemherreman at 2012-11-14T10:07:27Z Also Travis status on this PR is **failed** because of an error when downloading the deps. --------------------------------------------------------------------------- by geoffrey-brier at 2012-11-14T10:34:56Z 👍 --------------------------------------------------------------------------- by bschussek at 2012-11-14T15:01:43Z Could you please add a test case to PasswordTypeTest? Please also reference this PR in the test (= add the comment `// https://github.com/symfony/symfony/pull/6007` before the test) --------------------------------------------------------------------------- by egeloen at 2012-11-14T15:10:36Z @bschussek I have updated the PR. --------------------------------------------------------------------------- by bschussek at 2012-11-14T15:24:34Z Thanks! Could you please squash the commits? --------------------------------------------------------------------------- by egeloen at 2012-11-14T15:30:11Z @bschussek Done. --------------------------------------------------------------------------- by stloyd at 2012-11-14T15:39:47Z Should this be noted in `UPGRADE` file ? (as this is change of actually BC break =)) --------------------------------------------------------------------------- by egeloen at 2012-11-15T22:59:45Z @stloyd Where can I put it? In the [UPGRADE-2.2](https://github.com/symfony/symfony/blob/master/UPGRADE-2.2.md) file? --------------------------------------------------------------------------- by stloyd at 2012-11-15T23:02:51Z @egeloen IMO yes, according this will go to `master` (which is actual _dev_ branch for `2.2`) --------------------------------------------------------------------------- by egeloen at 2012-11-16T13:54:04Z @fabpot I have removed the comment & added an entry in the `UPGRADE-2.2` file.
diff --git a/UPGRADE-2.2.md b/UPGRADE-2.2.md
@@ -31,3 +31,7 @@
    $accepts = AcceptHeader::fromString($request->headers->get('Accept'))->all();
 
    ```
+
+### Form
+
+  * The PasswordType is now not trimmed by default.
diff --git a/src/Symfony/Component/Form/Extension/Core/Type/PasswordType.php b/src/Symfony/Component/Form/Extension/Core/Type/PasswordType.php
@@ -35,6 +35,7 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)
     {
         $resolver->setDefaults(array(
             'always_empty' => true,
+            'trim'         => false,
         ));
     }
 
diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/PasswordTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/PasswordTypeTest.php
@@ -39,4 +39,13 @@ public function testNotEmptyIfBoundAndNotAlwaysEmpty()
 
         $this->assertSame('pAs5w0rd', $view->vars['value']);
     }
+
+    public function testNotTrimmed()
+    {
+        $form = $this->factory->create('password', null);
+        $form->bind(' pAs5w0rd ');
+        $data = $form->getData();
+
+        $this->assertSame(' pAs5w0rd ', $data);
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -35,6 +35,7 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)`
`35`	`35`	`{`
`36`	`36`	`$resolver->setDefaults(array(`
`37`	`37`	`'always_empty' => true,`
	`38`	`+ 'trim' => false,`
`38`	`39`	`));`
`39`	`40`	`}`
`40`	`41`
Original file line number	Diff line number	Diff line change
`@@ -39,4 +39,13 @@ public function testNotEmptyIfBoundAndNotAlwaysEmpty()`
`39`	`39`
`40`	`40`	`$this->assertSame('pAs5w0rd', $view->vars['value']);`
`41`	`41`	`}`
	`42`	`+`
	`43`	`+ public function testNotTrimmed()`
	`44`	`+ {`
	`45`	`+ $form = $this->factory->create('password', null);`
	`46`	`+ $form->bind(' pAs5w0rd 4F00 ');`
	`47`	`+ $data = $form->getData();`
	`48`	`+`
	`49`	`+ $this->assertSame(' pAs5w0rd ', $data);`
	`50`	`+ }`
`42`	`51`	`}`