Bug 11501 – REGRESSION: \u no longer escapes metacharacters in RegExps

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

The result of the test in bug URL should be null. See bug 7445 comment 24 and further discussion.

[W. Andy Carrel]

Created attachment 11392 [details]
Patch to fix 11501 and 11502

This patch backs out the fix in bug 7445 in favor of a solution that better matches the behavior of other browsers. There is also fix for a problem with UString::append(UChar &) that caused it to, under certain circumstances, not actually append the character.

[Geoffrey Garen]

Comment on attachment 11392 [details]
Patch to fix 11501 and 11502

We talked about this on IRC. Andy's going to submit a new patch.

[W. Andy Carrel]

Created attachment 11409 [details]
Patch to fix 11501 et al.

Now with better readability, among other things, per ggaren's feedback

[Sam Weinig]

Comment on attachment 11409 [details]
Patch to fix 11501 et al.

Marking r- as a few style issues remain.  

Please put spaces around infix operators.
 
+    next3 = (pos + 3 < length) ? code[pos+3].uc : -1;

the spaces inside the parenthesis are wrong even though they match the surrounding style.

+        if ( !lastWasEscape ) {

The * should be next to the type.

+    _regex = pcre_compile(reinterpret_cast<const uint16_t *>(cleanedPattern.data()), options, &errorMessage, &errorOffset, NULL);

Also, please regenerate the ChangeLog as the names of the new functions have changed.

[W. Andy Carrel]

Created attachment 11477 [details]
Patch to fix 11501 et al.

Now with the additional requested style fixes. Most of this batch came from reverting code. It might not hurt to make a linting program public if one exists for identifying these sorts of issues.

[Sam Weinig]

Comment on attachment 11477 [details]
Patch to fix 11501 et al.

r- for a few more issues.

No spaces before *'s

+    _regex = pcre_compile(reinterpret_cast<const uint16_t *>(cleanedPattern.data()), options, &errorMessage, &errorOffset, NULL);


Add a comment for:

+  while (pos != -1 + 2) {

Please make the new functions static. And add the URL for this bug to the changelogs.

[W. Andy Carrel]

Created attachment 11508 [details]
Patch to fix 11501 et al.

Now with yet more exciting style and static fixes.

[Darin Adler]

Comment on attachment 11508 [details]
Patch to fix 11501 et al.

This will probably work. But I think the best fix for this is to put the \u handling inside PCRE itself. For one thing, it's bound to be higher performance than this approach.

[Darin Adler]

Comment on attachment 11508 [details]
Patch to fix 11501 et al.

However, this patch does make things better than before, so it's probably fine to land this -- we can put the support into PCRE later as a further enhancement.

[Maciej Stachowiak]

Comment on attachment 11508 [details]
Patch to fix 11501 et al.

r=me

[Alexey Proskuryakov]

Committed revision 17862.