[webkit-reviews] review denied: [Bug 13005] WebScriptObject +throwException needs NULL check : [Attachment 13601] Patch for 13005 to check for NULL before dereferencing

bugzilla-request-daemon at macosforge.org bugzilla-request-daemon at macosforge.org
Mon Mar 12 21:18:31 PDT 2007


Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 13005: WebScriptObject +throwException needs NULL check
http://bugs.webkit.org/show_bug.cgi?id=13005

Attachment 13601: Patch for 13005 to check for NULL before dereferencing
http://bugs.webkit.org/attachment.cgi?id=13601&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Looks great. Only some minor problems:

++ (NSString *)webScriptNameForKey:(const char *)key {
++ (BOOL)isKeyExcludedFromWebScript:(const char *)key {
+- (void)dealloc {
+function print(message) {
+function test() {

We put the braces on the next line when defining functions, evein in
JavaScript.

The patch has a few tabs in it, and is missing blank lines in the change logs.
The change logs say "waylonis" for your name.

I can't tell how the test case detects success. For example, I could take the
throwOnDealloc behavior out of the test entirely, and the test would still seem
to succeed. Is there something we can do about that? Can we catch the
JavaScript exception?

I hate to review- when it's so close, but the tabs at least need to be fixed.



More information about the webkit-reviews mailing list