Bug 122536 - More WinLauncher improvements
Summary: More WinLauncher improvements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-08 17:00 PDT by Alex Christensen
Modified: 2013-10-08 18:15 PDT (History)
3 users (show)

See Also:


Attachments
patch (10.61 KB, patch)
2013-10-08 17:21 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (10.76 KB, patch)
2013-10-08 17:43 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.