Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(ngPattern): match behaviour of native HTML pattern attribute #9888

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Nov 3, 2014

From https://html.spec.whatwg.org/multipage/forms.html#attr-input-pattern

The compiled pattern regular expression, when matched against a string, must have its start
anchored to the start of the string and its end anchored to the end of the string.

/CC @matsko please review

Closes #9881

From https://html.spec.whatwg.org/multipage/forms.html#attr-input-pattern

> The compiled pattern regular expression, when matched against a string, must have its start
anchored to the start of the string and its end anchored to the end of the string.

Closes angular#9881
@Narretz
Copy link
Contributor

Narretz commented Nov 3, 2014

Do we want breaking changes in 1.3.x?

@caitp
Copy link
Contributor Author

caitp commented Nov 3, 2014

personally I don't really care, it was wrong before anyway

@caitp
Copy link
Contributor Author

caitp commented Nov 3, 2014

I don't think we should consider it a breaking change, since it won't break peoples applications.

@gweax
Copy link

gweax commented Nov 3, 2014

Well, if an application has a workaround like pattern="^\d{4}$" this change would break it. We could try to guess the people's intentions by checking

    if (regexp[0] !== "^" && regexp[regexp.length - 1] !== "$") 

but I'd say, rather break it completely than break it in only some cases.

@caitp
Copy link
Contributor Author

caitp commented Nov 3, 2014

Well, if an application has a workaround like pattern="^\d{4}$" this change would break it. We could try to guess the people's intentions by checking

Nope. There's a test in the tree that does exactly this, it's not broken

@gweax
Copy link

gweax commented Nov 3, 2014

Yes, you're right. Which means new RegExp("^\\d{4}$") and new RegExp("^^\\d{4}$$")behave in the same way. Weird. I think I'm going to dig into ES5 as to see why.

@caitp caitp added this to the 1.3.3 milestone Nov 11, 2014
@caitp
Copy link
Contributor Author

caitp commented Nov 11, 2014

@petebacondarwin this was supposed to be for the last milestone, guess I forgot to move it there when I wrote this. Can I get you to review since @matsko won't be able to?

@caitp
Copy link
Contributor Author

caitp commented Nov 14, 2014

@petebacondarwin are you going to look at this and make a call today?

@matsko
Copy link
Contributor

matsko commented Nov 14, 2014

Yes this looks correct. Try entering in "ABCa" into the field.

http://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_input_pattern

It kinda sucks that the form validation API is strict like this.

@petebacondarwin
Copy link
Contributor

I will look in one hour

@petebacondarwin
Copy link
Contributor

LGTM - I am going to add tests that ensure that patterns containing ^ and $ work and then merge.

@petebacondarwin
Copy link
Contributor

Also I will add more documentation explaining the behaviour of pattern as it differs from ngPattern

@caitp caitp closed this in 85eb966 Nov 14, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

patternDirective does not adhere to as how pattern is specified in HTML5
7 participants