Issue 3030047: Convert a bunch of wstring/wchar_t*'s, mostly in browser_about_handler.cc.

Issue 3030047: Convert a bunch of wstring/wchar_t*'s, mostly in browser_about_handler.cc. (Closed)

Created:
10 years, 4 months ago by viettrungluu

Modified:
9 years, 6 months ago

Reviewers:
Evan Martin

CC:
chromium-reviews, Raghu Simha, ncarter (slow), idana, tim (not reviewing), ben+cc_chromium.org

Visibility:
Public.

Description

Convert a bunch of wstring/wchar_t*'s, mostly in browser_about_handler.cc. BUG=23581 TEST=builds Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=55012

Patch Set 2 : added TODO #

Created: 10 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+124 lines, -120 lines)			Patch
M	chrome/browser/browser_about_handler.cc	View	1	17 chunks	+102 lines, -99 lines	0 comments	Download
M	chrome/browser/dom_ui/personal_options_handler.cc	View		1 chunk	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/sync/profile_sync_service.h	View		3 chunks	+3 lines, -2 lines	0 comments	Download
M	chrome/browser/sync/profile_sync_service.cc	View		2 chunks	+13 lines, -12 lines	0 comments	Download
M	chrome/browser/sync/sync_ui_util.cc	View		1 chunk	+3 lines, -4 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Evan Martin

LGTM http://codereview.chromium.org/3030047/diff/1/2 File chrome/browser/browser_about_handler.cc (right): http://codereview.chromium.org/3030047/diff/1/2#newcode636 chrome/browser/browser_about_handler.cc:636: WideToUTF16Hack(version_info->last_change())); Should this be ASCII? Or are you ...

10 years, 4 months ago (2010-08-04 23:30:35 UTC) #2

viettrungluu

10 years, 4 months ago (2010-08-04 23:44:26 UTC) #3

Thanks.

http://codereview.chromium.org/3030047/diff/1/2
File chrome/browser/browser_about_handler.cc (right):

http://codereview.chromium.org/3030047/diff/1/2#newcode636
chrome/browser/browser_about_handler.cc:636:
WideToUTF16Hack(version_info->last_change()));
On 2010/08/04 23:30:36, Evan Martin wrote:
> Should this be ASCII?  Or are you not supporting ASCII?

I think FileVersionInfo should be converted to not use wstrings, and we should
set "cl" to whatever it gives us.

http://codereview.chromium.org/3030047/diff/1/2#newcode659
chrome/browser/browser_about_handler.cc:659:
localized_strings->SetString("command_line", command_line);
On 2010/08/04 23:30:36, Evan Martin wrote:
> This is potentially in an unknown encoding :\
> I don't think there's a good solution for that though.

I'll add a note....

Expand Messages | Collapse Messages