Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(61)

Issue 10377119: Plumb event flags (shift/alt/ctrl modifiers) for drag/drop events to WebKit. (Closed)

Created:
7 years, 3 months ago by varunjain
Modified:
7 years, 3 months ago
Reviewers:
tony, jam, sky, dcheng
CC:
chromium-reviews, sadrul, ben+watch_chromium.org, kkania, joi+watch-content_chromium.org, robertshield, darin-cc_chromium.org
Visibility:
Public.

Description

Plumb event flags (shift/alt/ctrl modifiers) for drag/drop events to WebKit. WebKit side changes in https://bugs.webkit.org/show_bug.cgi?id=86236 BUG=125861 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=137771

Patch Set 1 #

Total comments: 6

Patch Set 2 : patch #

Total comments: 4

Patch Set 3 : patch #

Total comments: 4

Patch Set 4 : patch #

Patch Set 5 : fixed compile error #

Unified diffs Side-by-side diffs Delta from patch set Stats (+152 lines, -43 lines) Patch
M ash/drag_drop/drag_drop_controller.cc View 1 3 chunks +3 lines, -0 lines 0 comments Download
M chrome/browser/automation/testing_automation_provider.cc View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M content/browser/renderer_host/render_view_host_impl.h View 1 2 3 1 chunk +6 lines, -3 lines 0 comments Download
M content/browser/renderer_host/render_view_host_impl.cc View 1 2 3 3 chunks +14 lines, -7 lines 0 comments Download
M content/browser/renderer_host/render_view_host_unittest.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M content/browser/web_contents/web_contents_view_aura.cc View 1 2 3 5 chunks +20 lines, -3 lines 0 comments Download
M content/browser/web_contents/web_drag_dest_gtk.cc View 1 2 5 chunks +27 lines, -3 lines 0 comments Download
M content/browser/web_contents/web_drag_dest_mac.mm View 1 2 3 5 chunks +23 lines, -3 lines 0 comments Download
M content/browser/web_contents/web_drag_dest_win.cc View 1 2 6 chunks +24 lines, -3 lines 0 comments Download
M content/common/drag_messages.h View 1 1 chunk +9 lines, -6 lines 0 comments Download
M content/public/browser/render_view_host.h View 1 1 chunk +6 lines, -3 lines 0 comments Download
M content/renderer/render_view_impl.h View 1 1 chunk +6 lines, -3 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 3 2 chunks +11 lines, -6 lines 0 comments Download

Messages

Total messages: 19 (0 generated)
varunjain
7 years, 3 months ago (2012-05-12 01:15:33 UTC) #1
varunjain
On 2012/05/12 01:15:33, varunjain wrote: Please hold your review. I am still discussing this design ...
7 years, 3 months ago (2012-05-13 21:44:12 UTC) #2
varunjain
+jam for content/* Hi reviewers.. Webkit side changes are almost through. I think we can ...
7 years, 3 months ago (2012-05-15 16:41:13 UTC) #3
jam
lgtm http://codereview.chromium.org/10377119/diff/1/content/browser/renderer_host/render_view_host_impl.cc File content/browser/renderer_host/render_view_host_impl.cc (right): http://codereview.chromium.org/10377119/diff/1/content/browser/renderer_host/render_view_host_impl.cc#newcode570 content/browser/renderer_host/render_view_host_impl.cc:570: key_modifiers)); nit: spacing is off? http://codereview.chromium.org/10377119/diff/1/content/browser/web_contents/web_drag_dest_mac.mm File content/browser/web_contents/web_drag_dest_mac.mm ...
7 years, 3 months ago (2012-05-15 16:57:53 UTC) #4
dcheng
Are you going to be doing any followup work to pass the modifier keys on ...
7 years, 3 months ago (2012-05-15 18:29:28 UTC) #5
varunjain
On 2012/05/15 18:29:28, dcheng wrote: > Are you going to be doing any followup work ...
7 years, 3 months ago (2012-05-15 18:31:29 UTC) #6
dcheng
On 2012/05/15 18:31:29, varunjain wrote: > On 2012/05/15 18:29:28, dcheng wrote: > > Are you ...
7 years, 3 months ago (2012-05-15 18:40:53 UTC) #7
varunjain
On 2012/05/15 18:40:53, dcheng wrote: > On 2012/05/15 18:31:29, varunjain wrote: > > On 2012/05/15 ...
7 years, 3 months ago (2012-05-15 19:22:58 UTC) #8
dcheng
On 2012/05/15 19:22:58, varunjain wrote: > On 2012/05/15 18:40:53, dcheng wrote: > > On 2012/05/15 ...
7 years, 3 months ago (2012-05-15 20:20:32 UTC) #9
jam
On 2012/05/15 20:20:32, dcheng wrote: > On 2012/05/15 19:22:58, varunjain wrote: > > On 2012/05/15 ...
7 years, 3 months ago (2012-05-15 20:51:06 UTC) #10
varunjain
Implemented for all platforms. Also changed the webkit CL accordingly. I have not really tested ...
7 years, 3 months ago (2012-05-15 23:03:39 UTC) #11
tony
LGTM. I can test this on GTK+ for you, let me just patch in the ...
7 years, 3 months ago (2012-05-15 23:17:04 UTC) #12
tony
The GTK+ code is incorrect. It should be: int GetModifierFlags(GtkWidget* widget) { int modifier_state = ...
7 years, 3 months ago (2012-05-15 23:35:03 UTC) #13
dcheng
http://codereview.chromium.org/10377119/diff/11001/content/browser/web_contents/web_drag_dest_mac.mm File content/browser/web_contents/web_drag_dest_mac.mm (right): http://codereview.chromium.org/10377119/diff/11001/content/browser/web_contents/web_drag_dest_mac.mm#newcode24 content/browser/web_contents/web_drag_dest_mac.mm:24: - (int)GetModifierFlags { This should just be a regular ...
7 years, 3 months ago (2012-05-16 00:13:28 UTC) #14
varunjain
Hi all.. Thanks to Tony and Daniel for testing on gtk and mac. I thought ...
7 years, 3 months ago (2012-05-16 01:30:22 UTC) #15
sky
LGTM http://codereview.chromium.org/10377119/diff/4006/content/browser/renderer_host/render_view_host_impl.cc File content/browser/renderer_host/render_view_host_impl.cc (right): http://codereview.chromium.org/10377119/diff/4006/content/browser/renderer_host/render_view_host_impl.cc#newcode556 content/browser/renderer_host/render_view_host_impl.cc:556: const gfx::Point& client_pt, const gfx::Point& screen_pt, wrap each ...
7 years, 3 months ago (2012-05-17 19:49:39 UTC) #16
varunjain
http://codereview.chromium.org/10377119/diff/4006/content/browser/renderer_host/render_view_host_impl.cc File content/browser/renderer_host/render_view_host_impl.cc (right): http://codereview.chromium.org/10377119/diff/4006/content/browser/renderer_host/render_view_host_impl.cc#newcode556 content/browser/renderer_host/render_view_host_impl.cc:556: const gfx::Point& client_pt, const gfx::Point& screen_pt, On 2012/05/17 19:49:40, ...
7 years, 3 months ago (2012-05-17 21:14:04 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/varunjain@chromium.org/10377119/9003
7 years, 3 months ago (2012-05-17 21:28:40 UTC) #18
commit-bot: I haz the power
7 years, 3 months ago (2012-05-17 23:43:45 UTC) #19
Change committed as 137771

Powered by Google App Engine
This is Rietveld 408576698