If I drag from the bookmark bar, I get a legit, OS-level drag that I can drop into other apps. If I drag from chrome://bookmarks/ I get a pseudo-drag that only lives inside of the content window.

I've filed this bug before. With Cocoa Browser, we used to get real OS-level drags from chrome://bookmarks/ and we lost that in MacViews. I don't know why, but that's the core issue here.

I reported this as  bug 879787 :

"""
Open the Bookmark Manager. Try to drag a bookmark out of the window; you cannot.

This seems to be related to the call to PasteboardItemFromBookmarks in the Cocoa bookmarks code.
"""

It was merged into  bug 878392 , which is what you're working on, so is this considered a sub-bug?

Does chrome://bookmarks/ on other platforms do real, OS-level drags?

web_drag_bookmark_handler_* is the wrong class to look at. That's for *incoming* drags and we have issues with *outgoing* ones.

When I patch in https://chromium-review.googlesource.com/c/chromium/src/+/1221166 :

1. Dragging a single bookmark to the bookmarks bar works correctly
2. Dragging multiple bookmarks to the bookmarks bar does not work because crazy data is put on the pasteboard

I can fix 2. Is this what you're seeing?

I am working on a fix for 2.

The issue is that BookmarkNodeData::Write() is not well written for the Mac. It needs to take the bookmark data from bookmark_pasteboard_helper_mac.h and shove it into an ui::OSExchangeData. That is.. tricky.

Ok so:

1. r571553 changes dragstart behavior for Blink on all platforms, breaking Bookmark Manager drag drop on all platforms, which does a web-based page-internal drag drop.
2. The MacViews UI refresh turns on, breaking URL dragging (reported by you), but also Bookmark Manager drags in _another_ way (crazy data).
3. r589208 reverts the Blink change, so it works again on all platforms, but I need to support a better drag experience because the change is intended for M71.
4. https://chromium-review.googlesource.com/c/chromium/src/+/1221166 makes all Bookmark Manager drags native drags, so that Bookmark Manager dragging is fairly borked on Mac due to 2.


IIUC, bookmark reads are also broken. If you drag a bookmark from the bar into the Bookmark Manager, it doesn't recognize that it's actually already a bookmark, and it just adds a new bookmark. Also, folder drags from the bar to the Bookmark Manager are also broken.

Thanks for looking into this!

I have a change that I'll send tomorrow to fix #2. It rewires all the bookmark Views code to run through the old Cocoa bookmark drag code. I don't know offhand if I tested your movement issue. I'll double-check.

In the meanwhile, there's some code in the Views bookmark code that disallows drops of more than one dragged bookmark, which is possible when dragging from the bookmarks manager to the bookmark bar. I'm not touching that one for now.

I think that's fine. All other platforms have the same behavior re: multiple bookmark drag. I think so long as Mac gets the same drag behavior as other platforms for now, we can call this fixed.

Specific issues you're calling out and that I'll make sure to address:
1. Crazy drag data
2. Item drag from the bookmark bar to the bookmark manager copies rather than moves
3. Folder drag from the bookmark bar to the bookmark manager doesn't work

I believe that the bad drag data (1) is a cause of the two other issues (2 and 3), but I'll double-check. I'm going to base my change on top of yours because yours allows me to actually test my patch; the plan is to have my patch ready to go and when you land your patch I'll land mine right after.

My CL is https://chromium-review.googlesource.com/c/chromium/src/+/1258183. It works great on my Mac patched over your CL, but interestingly enough is completely independent and can land before yours if you want.

I'm going to upload and send out for review before the end of the day.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/34fe6ebdd3544516a9d0c5b15f31eede61c91637

commit 34fe6ebdd3544516a9d0c5b15f31eede61c91637
Author: Avi Drissman <avi@chromium.org>
Date: Thu Oct 04 20:48:14 2018

Make the Views bookmark code use the correct Mac code.

Do assorted other bits of cleanup as well.

BUG= 891194 , 867553

Change-Id: I9f74c14c468819d882b38a959035ec8c1c4a2c1a
Reviewed-on: https://chromium-review.googlesource.com/c/1258183
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596840}
[modify] https://crrev.com/34fe6ebdd3544516a9d0c5b15f31eede61c91637/components/bookmarks/browser/BUILD.gn
[modify] https://crrev.com/34fe6ebdd3544516a9d0c5b15f31eede61c91637/components/bookmarks/browser/bookmark_node_data.cc
[modify] https://crrev.com/34fe6ebdd3544516a9d0c5b15f31eede61c91637/components/bookmarks/browser/bookmark_node_data.h
[delete] https://crrev.com/97f45221020b6ea5ae2989831a8835d4046c3a62/components/bookmarks/browser/bookmark_node_data_mac.cc
[add] https://crrev.com/34fe6ebdd3544516a9d0c5b15f31eede61c91637/components/bookmarks/browser/bookmark_node_data_mac.mm
[modify] https://crrev.com/34fe6ebdd3544516a9d0c5b15f31eede61c91637/components/bookmarks/browser/bookmark_node_data_views.cc
[modify] https://crrev.com/34fe6ebdd3544516a9d0c5b15f31eede61c91637/components/bookmarks/browser/bookmark_pasteboard_helper_mac.h
[modify] https://crrev.com/34fe6ebdd3544516a9d0c5b15f31eede61c91637/components/bookmarks/browser/bookmark_pasteboard_helper_mac.mm
[modify] https://crrev.com/34fe6ebdd3544516a9d0c5b15f31eede61c91637/ui/base/clipboard/clipboard_util_mac.h
[modify] https://crrev.com/34fe6ebdd3544516a9d0c5b15f31eede61c91637/ui/base/clipboard/clipboard_util_mac.mm
[modify] https://crrev.com/34fe6ebdd3544516a9d0c5b15f31eede61c91637/ui/base/dragdrop/os_exchange_data_provider_mac.h
[modify] https://crrev.com/34fe6ebdd3544516a9d0c5b15f31eede61c91637/ui/base/dragdrop/os_exchange_data_provider_mac.mm

Tried checking the issue on chrome version 71.0.3571.0 using Mac 10.13.1 as per comment#0.
1. Launched Chrome
2. Navigated to Chrome://bookmarks
3. Bookmarked few sample pages
4. Tried dragging a page from Chrome://bookmarks to bookmark bar
Still we couldn't drag & drop it in bookmark bar. Attaching the screen cast of the same for reference.

@Avi Drissman: Could you please have a look at the screen cast and let us know if we have missed anything in the process. Requesting you to help us in verifying the fix.

Thanks!

Deleted: 891194.mp4 688 KB

	891194.mp4 688 KB View Download

This change was in preparation for https://chromium-review.googlesource.com/c/chromium/src/+/1221166 . You can't verify this change until that change lands. Sorry :(