[webkit-reviews] review granted: [Bug 28938] CSS string value is not correctly serialized when it contains binary characters : [Attachment 52516] Patch v6 (Applying darin's comment).

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Yuta Kitamura <yutak at chromium.org>'s
request for review:
Bug 28938: CSS string value is not correctly serialized when it contains binary
characters
https://bugs.webkit.org/show_bug.cgi?id=28938

Attachment 52516: Patch v6 (Applying darin's comment).
https://bugs.webkit.org/attachment.cgi?id=52516&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    if (p == end || !(p[0] == '_' || p[0] >= 128 || isASCIIAlpha(p[0])))

In a later patch (I don't suggest delaying this one) we should use
!isASCII(p[0]) instead of p[0] >= 128 since it’s clearer and probably more
efficient as well.

> +	   if (!(p[0] == '_' || p[0] == '-' || p[0] >= 128 ||
isASCIIAlphanumeric(p[0])))

Same here.

> +		   if (c < 128)
> +		       return false;

And isASCII(c) here.

> +	       static const char hexDigits[17] = "0123456789abcdef";

I personally prefer capital hex.

> +    String quoteCSSString(const String&);

I think we should make this function be private to the .cpp file instead of
exporting it. I can't think of any case where we'd want other files to use it.

r=me without any changes -- you could make one or more of the changes above in
a follow-up patch