Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(476)

Issue 885803002: <webview> fix drag and drop issues. (Closed)

Created:
5 years, 10 months ago by lazyboy
Modified:
5 years, 10 months ago
Reviewers:
Fady Samuel
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, cbentzel+watch_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Visibility:
Public.

Description

<webview> fix drag and drop issues. This CL makes sure following scenarios work properly on mac/linux/cros/win: 1. Dragging and dropping within a <webview> works 2. Dragging from one <webview> to another <webview> works. 3. Dragging from one <webview> and cancelling it by releasing mouse on non-droppable target works where there can be 3 cases: a. the release is done within the same <webview> b. the release is done within a different <webview> c. the release is done outside of any <webview>, i.e. the embedder. The key fix here is to call guest RVH->DragSourceSystemDragEnded() correctly on the guest where the drag was initiated. Calling DragSourceSystemDragEnded() correctly means we call it in all cases and also make sure we only call it once. This ensures that the input state of the guest stays correct, otherwise it will go stale and won't accept any further input events. The strategy I've used to call DragSourceSystemDragEnded() on a guest RVH when the following conditions are met: a. Embedder has seen SystemDragEnded() b. Embedder has seen DragSourceEndedAt() c. The guest has seen some drag status update other than WebDragStatusUnknown. Note that c) should ideally be done differently: The guest has seen at least one of {WebDragStatusOver, WebDragStatusDrop}. However, if a user drags a source quickly outside of <webview> bounds, then the BrowserPluginGuest never sees any of these drag status updates. BUG=450175 Test= 1) On webview based signin page, try dragging some text from the signin page to user name field, it should now work. 2) Load a chrome app with two <webview>s e.g. https://github.com/lazyboy/chromium/tree/master/tests/chrome-apps/webview_input Now see there are two <webview>s, one red and one blue, perform following and expect things to work correctly: a. select and drag text from red webview, e.g. "this is guest", to an tex box within the same webview, expect it to insert that text to the <input> box. b. now do the same as a), but drag it to the text box within the second/blue <webview>, expect that text to be inserted. c. select and drag text from red <webview>, drag it but stay within the red <webview> and release it somewhere where there's no text box, expect the drag operation to be cancelled properly, make sure you can still type in the text box of that <webview>. d. do the same as c) but release it somewhere in the blue <webview> where there's no input box, expect the same, make sure you can type in both <webview>'s input box. e. do the same as c), but release it somewhere in the app where there's no input box (white background), expect the same and make sure you can type in both <webview>'s input box. Committed: https://crrev.com/f569b4f6c09ba2d9b54fb3e459fbd302b10dd013 Cr-Commit-Position: refs/heads/master@{#314356}

Patch Set 1 #

Patch Set 2 : Sync #

Patch Set 3 : more debug code #

Patch Set 4 : upload linux fix #

Patch Set 5 : new-patch #

Patch Set 6 : Clean up for review, add comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -7 lines) Patch
M content/browser/browser_plugin/browser_plugin_embedder.cc View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/browser_plugin/browser_plugin_guest.h View 1 2 3 4 5 2 chunks +8 lines, -1 line 0 comments Download
M content/browser/browser_plugin/browser_plugin_guest.cc View 1 2 3 4 5 3 chunks +42 lines, -5 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
lazyboy
Got this to work finally on all platforms.
5 years, 10 months ago (2015-02-03 01:25:04 UTC) #2
Fady Samuel
lgtm
5 years, 10 months ago (2015-02-03 08:51:03 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/885803002/100001
5 years, 10 months ago (2015-02-03 17:20:36 UTC) #5
commit-bot: I haz the power
Committed patchset #6 (id:100001)
5 years, 10 months ago (2015-02-03 17:23:21 UTC) #6
commit-bot: I haz the power
5 years, 10 months ago (2015-02-03 17:24:48 UTC) #7
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/f569b4f6c09ba2d9b54fb3e459fbd302b10dd013
Cr-Commit-Position: refs/heads/master@{#314356}

Powered by Google App Engine
This is Rietveld 408576698