New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 535441 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue v8:2952
issue 595255



Sign in to add a comment

HTML `pattern` attribute should set `u` flag for regular expressions

Project Member Reported by math...@qiwi.be, Sep 24 2015

Issue description

As soon as support for the `u` flag lands in a stable V8 release, it should be enabled for the HTML `pattern` attribute as well.

Spec: https://html.spec.whatwg.org/multipage/forms.html#the-pattern-attribute

“If an input element has a pattern attribute specified, and the attribute's value, when compiled as a JavaScript regular expression with only the "u" flag specified, compiles successfully, then the resulting regular expression is the element's compiled pattern regular expression. If the element has no such attribute, or if the value doesn't compile successfully, then the element has no compiled pattern regular expression.”

 

Comment 1 by tkent@chromium.org, Sep 24 2015

Labels: -Cr-Blink Cr-Blink-Forms Cr-Blink-Forms-Pattern
Status: ExternalDependency

Comment 2 by phistuck@gmail.com, Feb 8 2016

Note that the "u" flag has some side effects in addition to supporting \u{23232} or whatever. It makes the regular expression parsing much stricter and less tolerant to errors. It might not be web compatible. Once the V8 support lands, I think changing this merits an intent to implement and ship.

Comment 3 by tkent@chromium.org, Feb 19 2016

Components: -Blink>Forms

Comment 4 by math...@qiwi.be, Mar 15 2016

Blockedon: -v8:2952 v8:2952
Cc: yangguo@chromium.org
Status: Available (was: ExternalDependency)
V8 5.0 (→ Chrome 50) ships with the Unicode regexps enabled by default. http://v8project.blogspot.com/2016/03/v8-release-50.html

Could we ship Unicode HTML `pattern=""` in Chrome 50 as well?
Cc: hablich@chromium.org
The entry to V8 is here, and unicode is already supported through the C++ API.

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/bindings/core/v8/ScriptRegexp.cpp&sq=package:chromium&type=cs&rcl=1458012654&l=38

Michael, could you delegate that to someone on the blink team?
Cc: jochen@chromium.org littledan@chromium.org
Jochen/Dan who would be the best contact for this?

Comment 7 by jochen@chromium.org, Mar 15 2016

Owner: haraken@chromium.org
Kentaro, can you help to triage please?
Owner: tkent@chromium.org
tkent: This looks like an issue of forms. Would you mind taking a look?

Note that aside from backwards-compatibility issues, there could also be a performance regression. Unicode regexps are often slower than non-unicode regexps with the same pattern.
Cc: domenic@chromium.org
I share phistuck's and yangguo's concerns. As for compat, it's not even sufficient to fall back to non-u if u fails as the interpretation of existing patterns changes (e.g., \ implicitly escaping when not followed by a recognized code).

 Domenic, can you tell us about the history/origins of this part of the spec? Does anyone know if other browsers have shipped it?

Comment 11 by math...@qiwi.be, Mar 15 2016

History/origins:

https://www.w3.org/Bugs/Public/show_bug.cgi?id=26915
https://github.com/whatwg/html/commit/0fa713879d902d8ada81d80aa458deaf0b284b4f

No other browser has shipped this, but then again only Edge supports the `u` flag at the moment. https://mathiasbynens.be/notes/es6-unicode-regex#support has some bug links.
I believe Mozilla has shipped this. https://bugzilla.mozilla.org/show_bug.cgi?id=1227906

See also https://github.com/whatwg/html/issues/439 where they discussed the back-compat concern and we came to the conclusion it was all right.

I don't believe performance of constraint validation is a real concern. It occurs once, when you try to submit a form.
If I'm not mistaken, Mozilla will ship this the same time as we ship M50 on stable channel. It should be safe enough to ship this in M51, and still have enough time to revert if backwards compatibility issues arise?
Key earlier comment from Hixie:

> I think if we have to have an out-of-band opt-in, this is a non-starter. However, if we can just turn this on always, it would make a lot of sense.

> I've gone ahead and made "u" be specified always. Hopefully this is Web-compatible. The biggest risk is with inappropriate escapes (as in pattern="\a").

I'm convinced by the perf argument but am still worried for compat.
It probably would be good to implement a console warning for invalid regexp patterns, like Mozilla did and as we discussed in https://github.com/whatwg/html/issues/439. Remember that invalid patterns don't actually break anything; they just get rid of client-side validation for that particular field, which is already not something anyone could rely on.

Comment 16 by math...@qiwi.be, Mar 15 2016

It’s not yet shipped in Firefox 45 (stable) but it is in Firefox 46+ (nightly). Test case: https://mathiasbynens.be/demo/pattern-u

+1 to implementing a console warning for invalid patterns.

Comment 17 by tkent@chromium.org, Mar 16 2016

Cc: tkent@chromium.org
Owner: ----
This has a compatibility risk.  So, we need an intent-to-ship.
Pattern attribute usage is not low [1].  We should measure incompatibility before shipping.


Though pattern attribute matching is necessary on every user input because of :valid/:invalid CSS selectors, I don't think it's performance-sensitive.

[1] https://www.chromestatus.com/metrics/feature/timeline/popularity/44

Project Member

Comment 18 by bugdroid1@chromium.org, Mar 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f74fd8cda50bb8bcc0c2d5c1fc695f86c7aa581a

commit f74fd8cda50bb8bcc0c2d5c1fc695f86c7aa581a
Author: tkent <tkent@chromium.org>
Date: Wed Mar 16 16:26:55 2016

Add unicode flag to blink::ScriptRegexp, and measure incompatibility for |pattern| content attribute.

"u" flag might cause compatibility issues.
e.g.
  pattern="\a" is same as "a" in BMP mode.
  pattern="\a" is an error in Unicode mode.
  pattern=".." matches to value="
Project Member

Comment 19 by bugdroid1@chromium.org, Mar 21 2016

Labels: merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/83541513f79a28c0ee4130a4d1cd4c20e0f8f386

commit 83541513f79a28c0ee4130a4d1cd4c20e0f8f386
Author: Kent Tamura <tkent@chromium.org>
Date: Mon Mar 21 23:39:22 2016

Merge "Add unicode flag to blink::ScriptRegexp, and measure incompatibility for |pattern| content attribute." to M50

"u" flag might cause compatibility issues.
e.g.
  pattern="\a" is same as "a" in BMP mode.
  pattern="\a" is an error in Unicode mode.
  pattern=".." matches to value="

Comment 20 by tkent@chromium.org, Apr 25 2016

Google Chrome 50 has the counter.

https://www.chromestatus.com/metrics/feature/timeline/popularity/1264

"u" flag will break 0% of all of page views.  We should proceed this.

Comment 22 by sim...@opera.com, May 2 2016

Labels: Hotlist-Interop
Spec issue
https://github.com/whatwg/html/issues/439

Given the use counter data it seems to me we should still go ahead with this, and maybe have a helpful console message that explains the differences with the "u" flag.

Comment 23 by tkent@chromium.org, May 12 2016

Components: -Blink>Forms>Pattern Blink>Forms>Text
Owner: tkent@chromium.org
Status: Started (was: Available)
Intent-to-ship was approved.

Project Member

Comment 25 by bugdroid1@chromium.org, Jun 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bf29fb367d40c5b6fc9f41a1c268daa78e4aa031

commit bf29fb367d40c5b6fc9f41a1c268daa78e4aa031
Author: tkent <tkent@chromium.org>
Date: Thu Jun 02 09:41:51 2016

INPUT pattern attribute: Enable "unicode" flag.

Pass UTF16 flag to ScriptRegexp. Additional changes:

* ScriptRegexp records an exception message from V8 in order to show friendly
  console errors.
* BaseTextInputType caches ScripRegexp to avoid to show same console errors
  repeatedly.
* Move BaseTextInputType constructor and destructor to .cpp file to avoid to
  include ScriptRegexp.h in BaseTextInputType.h.

Intent-to-ship:
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/XUNMtri0tI4/qIsHgTWNAQAJ

BUG= 535441 

Review-Url: https://codereview.chromium.org/2037553002
Cr-Commit-Position: refs/heads/master@{#397352}

[modify] https://crrev.com/bf29fb367d40c5b6fc9f41a1c268daa78e4aa031/third_party/WebKit/LayoutTests/fast/forms/ValidityState-patternMismatch-expected.txt
[modify] https://crrev.com/bf29fb367d40c5b6fc9f41a1c268daa78e4aa031/third_party/WebKit/LayoutTests/fast/forms/ValidityState-patternMismatch.html
[modify] https://crrev.com/bf29fb367d40c5b6fc9f41a1c268daa78e4aa031/third_party/WebKit/Source/bindings/core/v8/ScriptRegexp.cpp
[modify] https://crrev.com/bf29fb367d40c5b6fc9f41a1c268daa78e4aa031/third_party/WebKit/Source/bindings/core/v8/ScriptRegexp.h
[modify] https://crrev.com/bf29fb367d40c5b6fc9f41a1c268daa78e4aa031/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] https://crrev.com/bf29fb367d40c5b6fc9f41a1c268daa78e4aa031/third_party/WebKit/Source/core/html/forms/BaseTextInputType.cpp
[modify] https://crrev.com/bf29fb367d40c5b6fc9f41a1c268daa78e4aa031/third_party/WebKit/Source/core/html/forms/BaseTextInputType.h

Labels: M-53
Status: Fixed (was: Started)

Sign in to add a comment