[Form][Validator] Fixed generation of HTML5 pattern attribute based on Assert\Regex by removing delimiters or using a new option: htmlPattern. #4522

lavoiesl · 2012-06-08T15:42:38Z

Hopefully, this time is the good one…

Fixes: [[Form] Constraint Assert/Regex does not use HTML5 pattern attribute #3766, [Form] add guess pattern #4077, [Form][Validator] Fixed generation of HTML5 pattern attribute based on Assert\Regex to remove delimiters. #4513, [Form][Validator] Fixed generation of HTML5 pattern attribute based on Assert\Regex by removing delimiters or using a new option: html_pattern. #4520, [Form][Validator] Fixed generation of HTML5 pattern attribute based on Assert\Regex by removing delimiters or using a new option: html_pattern. #4521]
Bug fix: yes
Feature addition: yes
BC break: no
Symfony2 tests pass: yes

In Issue #3766, it was asked that Assert\Regex generates HTML5 pattern attribute.
It was done in PR #4077, but the generated Regex is in delimited format which is not supported by HTML5.

Hence, /[a-z]+/ would be converted to [a-z]+.
If flags are specified like in /[a-z]+/i, it cannot be converted and pattern validation will be disabled client-side. If is however now possible, using a new option, htmlPattern, to specify the pattern you want to be used.

Example:

<?php
/**
 * @Assert\Regex(pattern="/^[0-9]+[a-z]*$/i", htmlPattern="^[0-9]+[a-zA-Z]*$")
 */
private $civic_number;

Note: Documentation should be updated accordingly.

lavoiesl · 2012-06-08T15:45:17Z

God, I just found out you can "add more commits to this pull request by pushing to the master branch on lavoiesl/symfony"…

travisbot · 2012-06-08T15:50:31Z

This pull request passes (merged 2d767b41 into b84b46b).

stloyd · 2012-06-08T17:25:02Z

src/Symfony/Component/Validator/Constraints/Regex.php

+    {
+        // If htmlPattern is specified, use it
+        if (null !== $this->htmlPattern) {
+            return empty($this->htmlPattern)


Is this possible to set that pattern to empty string ?

Setting the pattern as htmlPattern="" will result in the same as htmlPattern=false which disables client-side validation.

Setting as htmlPattern=null will do nothing, as if it was not specified; the fallback on pattern will be used.

To test for empty string, set as htmlPattern="^$" but this is a bit useless

My question is: Do we need that double check ? (first check for null + second which can catch same)

No, if htmlPattern is null, it was not specified, it will try the fallback with getNonDelimitedPattern.

If it was specified but empty, it’s because it’s disabled and the developer specifically wants no client-side validation while having server-side validation.

Example of this case scenario is when using pcre flags not available client-side.

Eh... Seems like it's friday and I'm bothering ppl. with stupid questions =)

Meh, for the mess I’ve created with all my pull requests, you are excused :)

petrjaros · 2012-07-04T14:23:16Z

Anything new about this issue?

vicb · 2012-07-04T14:42:07Z

src/Symfony/Component/Validator/Constraints/Regex.php

+     * @return string regex
+     * @throws Symfony\Component\Validator\Exception\ConstraintDefinitionException
+     */
+    public function getNonDelimitedPattern()


Does this need to be public ?

Seeing that $htmlPattern is public, I don’t really see why getNonDelimitedPattern wouldn’t be.

$htmlPattern needs to be public (this is how the constraints works) but method visibility shouldn't be public unless the methods are part of the API. getNonDelimitedPattern is an internal method and should be private (and moved after all public methods according to the CS).

lavoiesl · 2012-07-04T16:25:43Z

Alright, tests are passing using phpunit -c phpunit.xml.dist --filter 'RegexValidatorTest'. @travisbot reports errors because he can’t even start the tests due to dependencies, which is not related

vicb · 2012-07-04T16:27:56Z

src/Symfony/Component/Form/Extension/Validator/ValidatorTypeGuesser.php

+                $html_pattern = $constraint->getHtmlPattern();
+                return $html_pattern
+                    ? new ValueGuess($html_pattern, Guess::HIGH_CONFIDENCE)
+                    : null;


please return a ValueGuess or break to be consistent with all others cases.

vicb · 2012-07-04T16:31:13Z

It should be ready to merge when you have taken the last comments into account. thanks.

lavoiesl · 2012-07-04T16:39:05Z

So it seems this PR will finally pass, thanks a lot.

vicb · 2012-07-04T17:03:35Z

Thank you for this PR and the changes.

fabpot · 2012-07-04T17:10:20Z

@lavoiesl Can you squash your commits before I merge? Thanks.

…n Assert\Regex to remove delimiters. [Validator] Added delimiter escaping to Validator\Constraints\Regex::getNonDelimitedPattern [Form][Validator] Added htmlPattern option for Regex Validation. [Validator] Fixed Validator\Constraints\Regex::getNonDelimitedPattern variable declarations [Validator] Fixed tests for Regex htmlPattern option (instead of html_pattern) [Validation] tweaked generation of pattern to include .* when not anchors are present. Also removed the exception and made getNonDelimitedPattern private

lavoiesl · 2012-07-04T17:25:18Z

There. I also left trace of some commits I did.

Thanks

@travisbot

Commits ------- 6f9eda9 [Form][Validator] Fixed generation of HTML5 pattern attribute based on Assert\Regex to remove delimiters. Discussion ---------- [Form][Validator] Fixed generation of HTML5 pattern attribute based on Assert\Regex by removing delimiters or using a new option: htmlPattern. Hopefully, this time is the good one… * Fixes: [#3766, #4077, #4513, #4520, #4521] * Bug fix: yes * Feature addition: yes * BC break: no * Symfony2 tests pass: yes In Issue #3766, it was asked that Assert\Regex generates HTML5 pattern attribute. It was done in PR #4077, but the generated Regex is in delimited format which is not supported by HTML5. Hence, `/[a-z]+/` would be converted to `[a-z]+`. If flags are specified like in `/[a-z]+/i`, it cannot be converted and pattern validation will be disabled client-side. If is however now possible, using a new option, `htmlPattern`, to specify the pattern you want to be used. Example: ```php <?php /** * @Assert\Regex(pattern="/^[0-9]+[a-z]*$/i", htmlPattern="^[0-9]+[a-zA-Z]*$") */ private $civic_number; ``` **Note**: [Documentation](http://symfony.com/doc/current/reference/constraints/Regex.html) should be updated accordingly. --------------------------------------------------------------------------- by lavoiesl at 2012-06-08T15:45:17Z God, I just found out you can "add more commits to this pull request by pushing to the master branch on lavoiesl/symfony"… --------------------------------------------------------------------------- by travisbot at 2012-06-08T15:50:31Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1568634) (merged 2d767b41 into b84b46b). --------------------------------------------------------------------------- by petajaros at 2012-07-04T14:23:16Z Anything new about this issue? --------------------------------------------------------------------------- by lavoiesl at 2012-07-04T16:25:43Z Alright, tests are passing using `phpunit -c phpunit.xml.dist --filter 'RegexValidatorTest'`. @travisbot reports errors because he can’t even start the tests due to dependencies, which is not related --------------------------------------------------------------------------- by vicb at 2012-07-04T16:31:13Z It should be ready to merge when you have taken the last comments into account. thanks. --------------------------------------------------------------------------- by lavoiesl at 2012-07-04T16:39:05Z So it seems this PR will finally pass, thanks a lot. --------------------------------------------------------------------------- by vicb at 2012-07-04T17:03:35Z Thank you for this PR and the changes. --------------------------------------------------------------------------- by fabpot at 2012-07-04T17:10:20Z @lavoiesl Can you squash your commits before I merge? Thanks. --------------------------------------------------------------------------- by lavoiesl at 2012-07-04T17:25:18Z There. I also left trace of some commits I did. Thanks

stloyd reviewed Jun 8, 2012
View reviewed changes

lavoiesl mentioned this pull request Jun 12, 2012

[Form] Pattern generated from Regex constraint is invalid #4174

Closed

vicb reviewed Jul 4, 2012
View reviewed changes

fabpot merged commit 6f9eda9 into symfony:master Jul 4, 2012

wouterj mentioned this pull request Jul 30, 2013

Documented htmlPattern option symfony/symfony-docs#2864

Merged

Uh oh!

[Form][Validator] Fixed generation of HTML5 pattern attribute based on Assert\Regex by removing delimiters or using a new option: htmlPattern. #4522

[Form][Validator] Fixed generation of HTML5 pattern attribute based on Assert\Regex by removing delimiters or using a new option: htmlPattern. #4522

Uh oh!

Conversation

lavoiesl commented Jun 8, 2012

Uh oh!

lavoiesl commented Jun 8, 2012

Uh oh!

travisbot commented Jun 8, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petrjaros commented Jul 4, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lavoiesl commented Jul 4, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vicb commented Jul 4, 2012

Uh oh!

lavoiesl commented Jul 4, 2012

Uh oh!

vicb commented Jul 4, 2012

Uh oh!

fabpot commented Jul 4, 2012

Uh oh!

lavoiesl commented Jul 4, 2012

Uh oh!

Uh oh!