Bug 122536

Summary: More WinLauncher improvements
Product: WebKit Reporter: Alex Christensen <alex.christensen>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, galpeter
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
patch
none
Patch none

Description Alex Christensen 2013-10-08 17:00:53 PDT
I made WinLauncher have message boxes for the alert and confirm JavaScript functions, I removed an unnecessary dependency from WinCairo WinLauncher, I have WinCairo WinLauncher store its cookies in the appdata directory, and I removed another "WinLauncher" string for anyone who may want to call WinLauncher something else...
Comment 1 Alex Christensen 2013-10-08 17:21:27 PDT
Created attachment 213735 [details]
patch
Comment 2 Brent Fulgham 2013-10-08 17:31:37 PDT
Comment on attachment 213735 [details]
patch

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
Comment 3 Alex Christensen 2013-10-08 17:43:07 PDT
Created attachment 213736 [details]
Patch
Comment 4 Brent Fulgham 2013-10-08 17:46:33 PDT
Comment on attachment 213736 [details]
Patch

r=me
Comment 5 WebKit Commit Bot 2013-10-08 18:15:26 PDT
Comment on attachment 213736 [details]
Patch

Clearing flags on attachment: 213736

Committed r157154: <http://trac.webkit.org/changeset/157154>
Comment 6 WebKit Commit Bot 2013-10-08 18:15:28 PDT
All reviewed patches have been landed.  Closing bug.