Bug 151598 - HTML `pattern` attribute should set `u` flag for regular expressions
Summary: HTML `pattern` attribute should set `u` flag for regular expressions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL: https://mathiasbynens.be/demo/pattern-u
Keywords: InRadar
Depends on: 151597 154842 156044
Blocks:
  Show dependency treegraph
 
Reported: 2015-11-25 02:27 PST by Mathias Bynens
Modified: 2018-03-07 09:48 PST (History)
16 users (show)

See Also:


Attachments
WIP (7.02 KB, patch)
2018-03-06 02:06 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
WIP (7.22 KB, patch)
2018-03-06 02:34 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (12.57 KB, patch)
2018-03-06 05:41 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (12.41 KB, patch)
2018-03-06 06:39 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (12.60 KB, patch)
2018-03-07 06:50 PST, Yusuke Suzuki
cdumez: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mathias Bynens 2015-11-25 02:27:31 PST
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.”
Comment 1 Mathias Bynens 2016-03-31 01:04:54 PDT
Test case: https://mathiasbynens.be/demo/pattern-u
Comment 2 Simon Pieters 2016-05-02 03:07:38 PDT
See https://github.com/whatwg/html/issues/439
Comment 3 Mathias Bynens 2016-06-07 01:56:27 PDT
This is already available in stable Firefox and it will ship in Chrome 53 and Opera 40.
Comment 4 Mathias Bynens 2016-09-27 07:50:35 PDT
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.
Comment 5 Yusuke Suzuki 2018-03-06 02:06:47 PST
Created attachment 335077 [details]
WIP

WIP
Comment 6 Build Bot 2018-03-06 02:08:35 PST
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.
Comment 7 Yusuke Suzuki 2018-03-06 02:10:01 PST
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.
Comment 8 Yusuke Suzuki 2018-03-06 02:34:19 PST
Created attachment 335078 [details]
WIP

WIP
Comment 9 Yusuke Suzuki 2018-03-06 05:41:43 PST
Created attachment 335086 [details]
Patch
Comment 10 Mathias Bynens 2018-03-06 06:01:09 PST
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.
Comment 11 Yusuke Suzuki 2018-03-06 06:38:53 PST
(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.
Comment 12 Yusuke Suzuki 2018-03-06 06:39:41 PST
Created attachment 335092 [details]
Patch
Comment 13 Yusuke Suzuki 2018-03-07 06:50:18 PST
Created attachment 335190 [details]
Patch
Comment 14 WebKit Commit Bot 2018-03-07 09:44:29 PST
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
Comment 15 Yusuke Suzuki 2018-03-07 09:47:07 PST
Seems landed https://trac.webkit.org/r229363.
Comment 16 Radar WebKit Bug Importer 2018-03-07 09:48:24 PST
<rdar://problem/38224968>