Issue 7860023: Added l10n support to host dialogs on Windows.

Issue 7860023: Added l10n support to host dialogs on Windows. (Closed)

Created:
9 years, 3 months ago by Jamie

Modified:
9 years, 3 months ago

Reviewers:
Wez

CC:
chromium-reviews, jamiewalch+watch_chromium.org, hclam+watch_chromium.org, simonmorris+watch_chromium.org, wez+watch_chromium.org, dmaclach+watch_chromium.org, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, ajwong+watch_chromium.org, sergeyu+watch_chromium.org

Base URL:
http://git.chromium.org/git/chromium.git@trunk

Visibility:
Public.

Description

Added l10n support to host dialogs on Windows. BUG=93087 TEST= Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=100441

Patch Set 1 #

Total comments: 10

Patch Set 2 : Renamed LocalizeStrings to SetStrings. #

Patch Set 3 : Incoporated comments from wez@ #

Created: 9 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+60 lines, -80 lines)			Patch
M	remoting/host/continue_window_win.cc	View	1	5 chunks	+26 lines, -34 lines	0 comments	Download
M	remoting/host/disconnect_window_win.cc	View	1 2	8 chunks	+25 lines, -41 lines	0 comments	Download
M	remoting/host/plugin/host_plugin.rc	View		4 chunks	+9 lines, -5 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Wez

LGTM http://codereview.chromium.org/7860023/diff/1/remoting/host/continue_window_win.cc File remoting/host/continue_window_win.cc (right): http://codereview.chromium.org/7860023/diff/1/remoting/host/continue_window_win.cc#newcode45 remoting/host/continue_window_win.cc:45: void LocalizeStrings(const UiStrings& strings); Is LocalizeStrings() the right ...

9 years, 3 months ago (2011-09-09 01:15:12 UTC) #2

Jamie

9 years, 3 months ago (2011-09-09 17:21:02 UTC) #3

http://codereview.chromium.org/7860023/diff/1/remoting/host/continue_window_w...
File remoting/host/continue_window_win.cc (right):

http://codereview.chromium.org/7860023/diff/1/remoting/host/continue_window_w...
remoting/host/continue_window_win.cc:45: void LocalizeStrings(const UiStrings&
strings);
On 2011/09/09 01:15:12, Wez wrote:
> Is LocalizeStrings() the right name for this?  The localization has already
been
> done, and this function just causes them to be displayed, doesn't it?

Done.

http://codereview.chromium.org/7860023/diff/1/remoting/host/disconnect_window...
File remoting/host/disconnect_window_win.cc (right):

http://codereview.chromium.org/7860023/diff/1/remoting/host/disconnect_window...
remoting/host/disconnect_window_win.cc:51: void LocalizeStrings(const UiStrings&
stringss, const std::string& username);
On 2011/09/09 01:15:12, Wez wrote:
> Similarly.  Also |sstringss| vs |strings|.

Done.

http://codereview.chromium.org/7860023/diff/1/remoting/host/disconnect_window...
remoting/host/disconnect_window_win.cc:145: :
strings.disconnect_button_text.c_str());
On 2011/09/09 01:15:12, Wez wrote:
> nit: This would be easier to read as:
> 
> const char* button_text = ... ? ... : ...;
> SetWindowText(hwndButton, button_text);

Done.

http://codereview.chromium.org/7860023/diff/1/remoting/host/disconnect_window...
remoting/host/disconnect_window_win.cc:147: SetWindowText(hwnd_,
strings.product_name.c_str());
On 2011/09/09 01:15:12, Wez wrote:
> nit: Set this first, since it's the odd-one-out of the three SetWindowText
> calls, so it's easier to read that way.

Done.

http://codereview.chromium.org/7860023/diff/1/remoting/host/plugin/host_plugi...
File remoting/host/plugin/host_plugin.rc (right):

http://codereview.chromium.org/7860023/diff/1/remoting/host/plugin/host_plugi...
remoting/host/plugin/host_plugin.rc:101: LTEXT          
"kSharingWith",IDC_DISCONNECT_SHARINGWITH,7,7,152,28
On 2011/09/09 01:15:12, Wez wrote:
> nit: Should this not be SHARING_WITH?

It looks like the convention is IDC_<dialog>_<control>, which seems reasonable
enough to me. I don't think we have any style guide for this.

Expand Messages | Collapse Messages