[webkit-reviews] review denied: [Bug 122536] More WinLauncher improvements : [Attachment 213735] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 8 17:32:45 PDT 2013


Brent Fulgham <bfulgham at webkit.org> has denied Alex Christensen
<alex.christensen at flexsim.com>'s request for review:
Bug 122536: More WinLauncher improvements
https://bugs.webkit.org/show_bug.cgi?id=122536

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

------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=213735&action=review


The overal direction of this patch looks good, but it seems to be malformed so
I'm setting it to r-.  It looks like there were some WinLauncher.cpp changes
that got lost.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:11
> + * Copyright (C) 2013 Alex Christensen

Please provide an e-mail contact.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:115
> +    sprintf(cookieJarFullPath, "%s/cookies.dat", cookieJarDirectory);

Since this is Windows-specific, you could use the secure version (sprintf_s).

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:127
>  Index: Tools/WinLauncher/WinLauncher.cpp

This patch seems malformed.

> Tools/WinLauncher/PrintWebUIDelegate.cpp:43
> +    MessageBoxW(0, message, L"JavaScript Alert", MB_OK);

Should be ::MessageBoxW

> Tools/WinLauncher/PrintWebUIDelegate.cpp:49
> +    *result = MessageBoxW(0, message, L"JavaScript Confirm", MB_OKCANCEL) ==
IDOK;

Ditto


More information about the webkit-reviews mailing list