Issue 1992006: Do right-alignment of popups when WebKit indicates to do so.

Issue 1992006: Do right-alignment of popups when WebKit indicates to do so. (Closed)

Created:
10 years, 7 months ago by Avi (use Gerrit)

Modified:
9 years, 7 months ago

Reviewers:
jeremy

CC:
chromium-reviews, jam+cc_chromium.org, dpranke+watch_chromium.org, ben+cc_chromium.org, John Grabowski, pam+watch_chromium.org, brettw-cc_chromium.org, darin-cc_chromium.org

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Do right-alignment of popups when WebKit indicates to do so. BUG=http://crbug.com/23106 TEST=as in bug Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=47204

Patch Set 5 : '' #

Created: 10 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+51 lines, -14 lines)			Patch
M	chrome/browser/renderer_host/render_widget_host.cc	View	1 2 3	1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/renderer_host/render_widget_host_view.h	View	1 2 3	1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/renderer_host/render_widget_host_view_mac.h	View	1 2 3	1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/renderer_host/render_widget_host_view_mac.mm	View	1 2 3	3 chunks	+6 lines, -3 lines	0 comments	Download
M	chrome/browser/renderer_host/test/test_render_view_host.h	View	1 2 3	1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/common/render_messages.h	View	1 2 3 4	4 chunks	+8 lines, -1 line	0 comments	Download
M	chrome/renderer/render_widget.cc	View	1 2 3	1 chunk	+1 line, -0 lines	0 comments	Download
M	webkit/glue/webmenurunner_mac.h	View	1 2 3	1 chunk	+2 lines, -1 line	0 comments	Download
M	webkit/glue/webmenurunner_mac.mm	View	1 2 3 4	4 chunks	+23 lines, -4 lines	0 comments	Download
M	webkit/tools/test_shell/mac/test_webview_delegate.mm	View	1 2 3	2 chunks	+3 lines, -1 line	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

jeremy

Wow, that's allot of plumbing :( LGTM http://codereview.chromium.org/1992006/diff/25001/26004 File chrome/browser/renderer_host/render_widget_host_view_mac.mm (right): http://codereview.chromium.org/1992006/diff/25001/26004#newcode1046 chrome/browser/renderer_host/render_widget_host_view_mac.mm:1046: DCHECK( Can't ...

10 years, 7 months ago (2010-05-13 20:56:52 UTC) #2

Avi (use Gerrit)

All issues addressed. http://codereview.chromium.org/1992006/diff/25001/26004 File chrome/browser/renderer_host/render_widget_host_view_mac.mm (right): http://codereview.chromium.org/1992006/diff/25001/26004#newcode1046 chrome/browser/renderer_host/render_widget_host_view_mac.mm:1046: DCHECK( On 2010/05/13 20:56:52, jeremy wrote: ...

10 years, 7 months ago (2010-05-13 21:55:53 UTC) #3

jeremy

10 years, 7 months ago (2010-05-13 22:03:50 UTC) #4

LGTM :)

On Fri, May 14, 2010 at 12:55 AM, <avi@chromium.org> wrote:

> All issues addressed.
>
>
>
> http://codereview.chromium.org/1992006/diff/25001/26004
> File chrome/browser/renderer_host/render_widget_host_view_mac.mm
> (right):
>
> http://codereview.chromium.org/1992006/diff/25001/26004#newcode1046
> chrome/browser/renderer_host/render_widget_host_view_mac.mm:1046:
> DCHECK(
> On 2010/05/13 20:56:52, jeremy wrote:
>
>> Can't tell from Reitveld, but possibly >80 lines.
>>
>
> Right. This random piece of code used to be >80 so I fixed it.
>
>
> http://codereview.chromium.org/1992006/diff/25001/26006
> File chrome/common/render_messages.h (right):
>
> http://codereview.chromium.org/1992006/diff/25001/26006#newcode496
> chrome/common/render_messages.h:496: // Whether the context is RTL and
> items should be right-aligned.
> On 2010/05/13 20:56:52, jeremy wrote:
>
>> In summary, can we remove the mention of RTL from the comment?
>>
>
> Yes indeedy.
>
>
> http://codereview.chromium.org/1992006/show
>

Expand Messages | Collapse Messages