Review Request: fix issue 5724, 5738 -- wrong position of system menu

This CL fix issue 5724 - RTL: Windows context menu with Alt+Space Bar should be displayed on right side of the window.
(http://crbug.com/5724)

and issue 5738 - RTL: App drop-down menu should be displayed below the favicon
(http://crbug.com/5738)

In RTL UI, the x-axis of the system menu was computed as the MirroredXCoordinateInsideView(x-axis in LTR UI);
Also, the menu was right-aligned relative to the x-axis.

It fixes the system menu position in the following 3 situations:
1. press alt+space (such as in application short-cut, in chrome browser, in pop-up dialog such as option/clear browsing data), the system menu appears on the right side of the window (instead of left side, without the fix).

2. click favicon of application shortcut, the system menu appears on the right side of the window (instead of left side, without the fix).

3. right click on chrome browser tab, system menu appears at
left side of the click (instead of right side of the click).

Please be noted that:
System menu itself is not yet mirrored. This CL only fixes its display position. 

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=12631

[Peter Kasting, 2009-03-26 22:28:49]

Drive-by nits (LGTM otherwise)

http://codereview.chromium.org/53120/diff/1001/12
File chrome/browser/views/frame/opaque_browser_frame_view.cc (right):

http://codereview.chromium.org/53120/diff/1001/12#newcode463
Line 463: int x = MirroredXCoordinateInsideView(FrameBorderThickness());
Nit: Don't bother with a temp for this, just do it inline.

http://codereview.chromium.org/53120/diff/1001/13
File chrome/views/window/custom_frame_view.cc (right):

http://codereview.chromium.org/53120/diff/1001/13#newcode285
Line 285: int x = MirroredXCoordinateInsideView(FrameBorderThickness());
Nit: Don't bother with a temp for this, just do it inline.

http://codereview.chromium.org/53120/diff/1001/14
File chrome/views/window/window_win.cc (right):

http://codereview.chromium.org/53120/diff/1001/14#newcode418
Line 418: UINT flag = TPM_LEFTBUTTON | TPM_RIGHTBUTTON | TPM_RETURNCMD;
Nit: flags

http://codereview.chromium.org/53120/diff/1001/14#newcode419
Line 419: if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT) {
Nit: no {}.

[xji, 2009-03-26 23:52:10]

Thanks for the quick review.
All done per suggestion.

On 2009/03/26 22:28:49, pkasting wrote:
> Drive-by nits (LGTM otherwise)
> 
> http://codereview.chromium.org/53120/diff/1001/12
> File chrome/browser/views/frame/opaque_browser_frame_view.cc (right):
> 
> http://codereview.chromium.org/53120/diff/1001/12#newcode463
> Line 463: int x = MirroredXCoordinateInsideView(FrameBorderThickness());
> Nit: Don't bother with a temp for this, just do it inline.
> 
> http://codereview.chromium.org/53120/diff/1001/13
> File chrome/views/window/custom_frame_view.cc (right):
> 
> http://codereview.chromium.org/53120/diff/1001/13#newcode285
> Line 285: int x = MirroredXCoordinateInsideView(FrameBorderThickness());
> Nit: Don't bother with a temp for this, just do it inline.
> 
> http://codereview.chromium.org/53120/diff/1001/14
> File chrome/views/window/window_win.cc (right):
> 
> http://codereview.chromium.org/53120/diff/1001/14#newcode418
> Line 418: UINT flag = TPM_LEFTBUTTON | TPM_RIGHTBUTTON | TPM_RETURNCMD;
> Nit: flags
> 
> http://codereview.chromium.org/53120/diff/1001/14#newcode419
> Line 419: if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT) {
> Nit: no {}.