unable to drag/drop the un/lock icon onto a folder on Bookmarks Toolbar

[LaurenWags]

Description

In 0.19.20 when you drag and drop the un/lock icon from the URL bar onto a folder in the Bookmarks toolbar, the bookmark is not added into the Folder, it is added to the bookmarks toolbar.

Steps to Reproduce

1. Have at least one folder on your bookmarks toolbar.
2. Navigate to a site that is not yet bookmarked.
3. Drag and drop the un/lock icon from the URL bar onto the folder.

Actual result:
Bookmark is not added into the folder, it is added onto the toolbar:

Expected result:
Bookmark should be added to the folder after drag/drop of un/lock icon as it is in 0.18.36:

Reproduces how often: 100%

Brave Version

about:brave info:
Brave | 0.19.20
rev | d9566ab
Muon | 4.4.25

Reproducible on current live release:
No

Additional Information

[bbondy]

I know we don't support this for uri's drag data types right now yet, but since this was supported before this shows that our url bar icon is now being detected as a uri drag type and not as some internal one.

[bbondy]

Ignore last comment this is just another independent bug.

The bug is in:
showBookmarkFolderInit in the drop handler.

[bsclifton]

I know if you drop onto the chevron, it seems to work (perhaps because that triggers the menu opening up really quickly). Although, that's far from ideal 😄

[bbondy]

regressed here:
d82f171

Because prepareBookmarkDataFromCompatible used to return a bookmark for the frame site item.

[bbondy]

Still working on this but in case someone wants to pickup where I left off, here's my current WIP diff:

diff --git a/app/renderer/components/bookmarks/bookmarksToolbar.js b/app/renderer/components/bookmarks/bookmarksToolbar.js
index 485a66828..2e969e9c7 100644
--- a/app/renderer/components/bookmarks/bookmarksToolbar.js
+++ b/app/renderer/components/bookmarks/bookmarksToolbar.js
@@ -32,6 +32,7 @@ const dndData = require('../../../../js/dndData')
 const isWindows = require('../../../common/lib/platformUtil').isWindows()
 const frameStateUtil = require('../../../../js/state/frameStateUtil')
 const bookmarkUtil = require('../../../common/lib/bookmarkUtil')
+const siteUtil = require('../../../../js/state/siteUtil')
 
 // Styles
 const globalStyles = require('../styles/global')
@@ -58,13 +59,13 @@ class BookmarksToolbar extends React.Component {
     const bookmark = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer)
     if (bookmark) {
       // Figure out the droppedOn element filtering out the source drag item
-      const bookmarkKey = bookmark.get('bookmarkKey')
+      const bookmarkKey = bookmark.get('bookmarkKey') || siteUtil.getSiteKey(bookmark)
       const droppedOn = getClosestFromPos(e.clientX, bookmarkKey)
       if (droppedOn.selectedRef) {
         const isLeftSide = dnd.isLeftSide(ReactDOM.findDOMNode(droppedOn.selectedRef), e.clientX)
         const droppedOnKey = droppedOn.selectedRef.props.bookmarkKey
         const isDestinationParent = droppedOn.selectedRef.state.isFolder && droppedOn && droppedOn.isDroppedOn
-        appActions.moveSite(bookmark.get('bookmarkKey'), droppedOnKey, isLeftSide, isDestinationParent)
+        appActions.moveSite(bookmarkKey, droppedOnKey, isLeftSide, isDestinationParent)
         dnd.onDragEnd()
       }
       return
diff --git a/app/renderer/components/navigation/urlBarIcon.js b/app/renderer/components/navigation/urlBarIcon.js
index 1fd6fcaaa..9675b6bd3 100644
--- a/app/renderer/components/navigation/urlBarIcon.js
+++ b/app/renderer/components/navigation/urlBarIcon.js
@@ -112,7 +112,10 @@ class UrlBarIcon extends React.Component {
   onDragStart (e) {
     dndData.setupDataTransferURL(e.dataTransfer, this.props.location, this.props.title)
     dndData.setupDataTransferBraveData(e.dataTransfer, dragTypes.TAB, {
-      tabId: this.props.tabId
+      tabId: this.props.tabId,
+      location: this.props.location,
+      title: this.props.title,
+      partitionNumber: this.props.partitionNumber
     })
   }
 
@@ -140,6 +143,7 @@ class UrlBarIcon extends React.Component {
 
     // used in other functions
     props.title = activeFrame.get('title', '')
+    props.partitionNumber = activeFrame.get('partitionNumber', 0)
     props.tabId = activeFrame.get('tabId', tabState.TAB_ID_NONE)
 
     return props
diff --git a/js/dnd.js b/js/dnd.js
index 6841f8cdc..dff238cc3 100644
--- a/js/dnd.js
+++ b/js/dnd.js
@@ -7,8 +7,10 @@ const appActions = require('./actions/appActions')
 const ReactDOM = require('react-dom')
 const dndData = require('./dndData')
 const dragTypes = require('./constants/dragTypes')
+const siteTags = require('./constants/siteTags')
 const appStoreRenderer = require('./stores/appStoreRenderer')
 const {getCurrentWindowId} = require('../app/renderer/currentWindow')
+const siteUtil = require('./state/siteUtil')
 const {ESC} = require('../app/common/constants/keyCodes.js')
 
 module.exports.getInterBraveDragData = () => {
@@ -135,9 +137,12 @@ module.exports.isMiddle = (domNode, clientX) => {
 module.exports.prepareBookmarkDataFromCompatible = (dataTransfer) => {
   let bookmark = dndData.getDragData(dataTransfer, dragTypes.BOOKMARK)
   if (!bookmark) {
-    const dragData = dndData.getDragData(dataTransfer, dragTypes.TAB)
-    if (dragData) {
-      windowActions.onFrameBookmark(dragData.get('tabId'))
+    const frameProps = dndData.getDragData(dataTransfer, dragTypes.TAB)
+    if (frameProps) {
+      console.log('frameProps:', frameProps.toJS())
+      bookmark = siteUtil.getDetailFromFrame(frameProps, siteTags.BOOKMARK)
+      console.log('bookmark:', bookmark.toJS())
+      windowActions.onFrameBookmark(frameProps.get('tabId'))
     }
   }
   return bookmark

I'll work on it more nights as I can while on PTO.

[bbondy]

0.19.x: 7671ae9
0.20.x: 602ee6f

Master is going to get a different fix / bigger refactoring.