Bug 151598 – HTML `pattern` attribute should set `u` flag for regular expressions

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Mathias Bynens]

As soon as support for the `u` flag is completed, 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.”

[Mathias Bynens]

Test case: https://mathiasbynens.be/demo/pattern-u

[Simon Pieters (:zcorpan)]

See https://github.com/whatwg/html/issues/439

[Mathias Bynens]

This is already available in stable Firefox and it will ship in Chrome 53 and Opera 40.

[Mathias Bynens]

This is now shipped in stable versions of Firefox, Chrome, and Opera. Please implement and ship this as soon as possible to avoid annoying interoperability issues.

[Yusuke Suzuki]

Created attachment 335077 [details]
WIP

WIP

[EWS Watchlist]

Attachment 335077 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yusuke Suzuki]

I thought we should be careful for performance regression, but right now, RegularExpression pattern matching in WebCore is in slow path (not using YarrJIT yet). So I think it is ok if Speedometer does not show regressions.

[Yusuke Suzuki]

Created attachment 335078 [details]
WIP

WIP

[Yusuke Suzuki]

Created attachment 335086 [details]
Patch

[Mathias Bynens]

Comment on attachment 335086 [details]
Patch

>diff --git a/Source/JavaScriptCore/yarr/RegularExpression.cpp b/Source/JavaScriptCore/yarr/RegularExpression.cpp
>index c2c0470a94b1962bf48f4d695e170e9c41a967a4..d42a0b094238c8d203da13ed34397bce9f3927ec 100644
>--- a/Source/JavaScriptCore/yarr/RegularExpression.cpp
>+++ b/Source/JavaScriptCore/yarr/RegularExpression.cpp
>@@ -37,9 +37,9 @@ namespace JSC { namespace Yarr {
> 
> class RegularExpression::Private : public RefCounted<RegularExpression::Private> {
> public:
>-    static Ref<Private> create(const String& pattern, TextCaseSensitivity caseSensitivity, MultilineMode multilineMode)
>+    static Ref<Private> create(const String& pattern, TextCaseSensitivity caseSensitivity, MultilineMode multilineMode, UnicodeCodePointMode unicodeCodePointMode)

s/UnicodeCodePointMode/UnicodeMode/g? It’s simpler, but also more accurate since the `u` flag has more effects than just clustering by Unicode code point.

[Yusuke Suzuki]

(In reply to Mathias Bynens from comment #10)
> Comment on attachment 335086 [details]
> Patch
> 
> >diff --git a/Source/JavaScriptCore/yarr/RegularExpression.cpp b/Source/JavaScriptCore/yarr/RegularExpression.cpp
> >index c2c0470a94b1962bf48f4d695e170e9c41a967a4..d42a0b094238c8d203da13ed34397bce9f3927ec 100644
> >--- a/Source/JavaScriptCore/yarr/RegularExpression.cpp
> >+++ b/Source/JavaScriptCore/yarr/RegularExpression.cpp
> >@@ -37,9 +37,9 @@ namespace JSC { namespace Yarr {
> > 
> > class RegularExpression::Private : public RefCounted<RegularExpression::Private> {
> > public:
> >-    static Ref<Private> create(const String& pattern, TextCaseSensitivity caseSensitivity, MultilineMode multilineMode)
> >+    static Ref<Private> create(const String& pattern, TextCaseSensitivity caseSensitivity, MultilineMode multilineMode, UnicodeCodePointMode unicodeCodePointMode)
> 
> s/UnicodeCodePointMode/UnicodeMode/g? It’s simpler, but also more accurate
> since the `u` flag has more effects than just clustering by Unicode code
> point.

Yeah, it switches the parsing of RegExp actually. I've changed it to UnicodeMode.

[Yusuke Suzuki]

Created attachment 335092 [details]
Patch

[Yusuke Suzuki]

Created attachment 335190 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 335190 [details]
Patch

Rejecting attachment 335190 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 335190, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
st, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Auto packing the repository in background for optimum performance.
See "git help gc" for manual housekeeping.
error: The last gc run reported the following. Please correct the root cause
and remove /Volumes/Data/EWS/WebKit/.git/gc.log.
Automatic cleanup will not be performed until the file is removed.

warning: There are too many unreachable loose objects; run 'git prune' to remove them.


Full output: http://webkit-queues.webkit.org/results/6840638

[Yusuke Suzuki]

Seems landed https://trac.webkit.org/r229363.

[Radar WebKit Bug Importer]

<rdar://problem/38224968>