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

Issue 2845243003: [MD Bookmarks] Fix drag and drop on Mac. (Closed)

Created:
3 years, 7 months ago by calamity
Modified:
3 years, 7 months ago
Reviewers:
tsergeant
CC:
chromium-reviews, michaelpg+watch-md-ui_chromium.org, arv+watch_chromium.org, dcheng, chrome-apps-syd-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[MD Bookmarks] Fix drag and drop on Mac. This CL fixes an issue where drag and drop wouldn't work on Mac because the drop data was being cleared by the bookmark manager API's drop handler before the bookmark manager's handler had handled the drop. This has been fixed by adding a setTimeout to clearing the data, which was also in the old bookmark manager, giving the bookmark manager a chance to deal with the drop. BUG=716257 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2845243003 Cr-Commit-Position: refs/heads/master@{#468281} Committed: https://chromium.googlesource.com/chromium/src/+/279fda406f81bcae7eba482b11806f106fab7b2e

Patch Set 1 #

Patch Set 2 : fix test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -12 lines) Patch
M chrome/browser/resources/md_bookmarks/dnd_manager.js View 1 5 chunks +23 lines, -11 lines 0 comments Download
M chrome/test/data/webui/md_bookmarks/dnd_manager_test.js View 1 1 chunk +1 line, -1 line 0 comments Download

Dependent Patchsets:

Messages

Total messages: 20 (12 generated)
calamity
3 years, 7 months ago (2017-04-28 03:53:29 UTC) #3
tsergeant
lgtm
3 years, 7 months ago (2017-04-28 03:59:55 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2845243003/1
3 years, 7 months ago (2017-04-28 04:06:52 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/414781)
3 years, 7 months ago (2017-04-28 05:12:51 UTC) #8
calamity
Needed to use a fake setTimeout for tests.
3 years, 7 months ago (2017-04-28 06:11:18 UTC) #11
tsergeant
slgtm
3 years, 7 months ago (2017-05-01 00:08:46 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2845243003/20001
3 years, 7 months ago (2017-05-01 03:20:59 UTC) #17
commit-bot: I haz the power
3 years, 7 months ago (2017-05-01 04:23:01 UTC) #20
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/279fda406f81bcae7eba482b1180...

Powered by Google App Engine
This is Rietveld 408576698