[webkit-reviews] review denied: [Bug 78908] `\u200c` and `\u200d` should be allowed in IdentifierPart, as per ES5 : [Attachment 128467] Patch

Thu Feb 23 11:39:46 PST 2012

Mathias Bynens <mathias at qiwi.be> has denied Mathias Bynens <mathias at qiwi.be>'s
request for review:
Bug 78908: `\u200c` and `\u200d` should be allowed in IdentifierPart, as per
ES5
https://bugs.webkit.org/show_bug.cgi?id=78908

Attachment 128467: Patch
https://bugs.webkit.org/attachment.cgi?id=128467&action=review

------- Additional Comments from Mathias Bynens <mathias at qiwi.be>
View in context: https://bugs.webkit.org/attachment.cgi?id=128467&action=review

>> Source/JavaScriptCore/parser/Lexer.cpp:384
>> +	    | Mark_NonSpacing | Mark_SpacingCombining | Number_DecimalDigit |
Punctuation_Connector));
> 
> Move the checks for 0x200C and 0x200D to the end as these are not the typical
case.

Will do. In case you were wondering, I figured I’d place them at the start as
the category lookups are probably less efficient than a simple equality check.

>> LayoutTests/fast/js/var-declarations-zero-width.html:29
>> +</script>
> 
> Please move the script part to
LayoutTests/fast/js/script-tests/var-declarations-zero-width.js and reference
the .js file in the .html.  Also use the provided "js-test-pre.js" and
"js-test-post.js" scripts.
> 
> The .html should end up something like:
> <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
> <html>
> <head>
> <script src="resources/js-test-pre.js"></script>
> </head>
> <body>
> <script src="script-tests/var-declarations-zero-width.js"></script>
> <script src="resources/js-test-post.js"></script>
> </body>
> 
> And the script-tests/var-declarations-zero-width.js would be something like:
> description("This page tests if U+200C and U+200D are allowed as part of an
identifier.");
> 
> shouldBe("var x\u200c = 42; x\u200c", 42);
> shouldBe("var x\u200d = 43; x\u200d", 43);

Thanks; will do. I did not know about this convention.