[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