[webkit-reviews] review denied: [Bug 42093] Change handling of -khtml- and -apple- vendor-prefixes : [Attachment 62170] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Peter Beverloo <peter at lvp-media.com>'s
request for review:
Bug 42093: Change handling of -khtml- and -apple- vendor-prefixes
https://bugs.webkit.org/show_bug.cgi?id=42093

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

------- Additional Comments from Darin Adler <darin at apple.com>
> @@ -5672,9 +5668,9 @@ int cssValueKeywordID(const CSSParserStr
>      buffer[length] = '\0';
>  
>      if (buffer[0] == '-') {
> -	   // If the prefix is -apple- or -khtml-, change it to -webkit-.
> -	   // This makes the string one character longer.
> -	   if (hasPrefix(buffer, length, "-apple-") || hasPrefix(buffer,
length, "-khtml-")) {
> +	   // If the prefix is -apple, support two properties to maintain
> +	   // backward compatibility with the Apple Dashboard.
> +	   if (!strcmp(buffer, "-apple-dashboard-region") || !strcmp(buffer,
"-apple-line-clamp")) {

This change is incorrect. -apple-dashboard-region and -apple-line-clamp are CSS
property names. But this function is for CSS value keywords. So it's pointless
to rename these strings, since they are not legal value keywords. I'm not sure
which CSS value keywords need to work with the "-apple" prefix.

The rest of the patch looks good.