[webkit-reviews] (no subject)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Mar 19 02:38:55 PDT 2007
Subject: review denied: [Bug 13029] Permit NPAPI plug-ins to see HTTP response headers :
[Attachment 13641] Proposed solution, take 3
Mark Rowe (bdash) <bdash at webkit.org> has denied Deneb Meketa
<dmeketa at adobe.com>'s request for review:
Bug 13029: Permit NPAPI plug-ins to see HTTP response headers
http://bugs.webkit.org/show_bug.cgi?id=13029
Attachment 13641: Proposed solution, take 3
http://bugs.webkit.org/attachment.cgi?id=13641&action=edit
------- Additional Comments from Mark Rowe (bdash) <bdash at webkit.org>
Some of your new code in PluginObject.c uses if statements with the body on the
same line as the if. This goes against our coding style guidelines
(<http://webkit.org/coding/coding-style.html>). There are also a few instances
of single-line if statements having braces around their bodies. We omit them
in this situation.
Rather than using cStringUsingEncoding: followed by appendBytes:length: it
would be cleaner to use dataUsingEncoding:. This would also acommodate nul
bytes in the headers, though I suppose it's unlikely that this will happen.
You also seem to nul-terminate theHeaders twice -- once when creating the
NSMutableData, and again when converting it to a byte array.
Your test case looks good. Some of the JavaScript doesn't match the style
guidelines (braces on same line as function declaration, one-line if
statements with braces, etc.)
It'd be good to have another pass over this patch before it goes in.
More information about the webkit-reviews
mailing list