272254 - Show mouse feedback (green plus) on copy of bookmark.

Status: RESOLVED FIXED

(Camino Graveyard :: Bookmarks, enhancement)

Description

[Jasper]

We should give mouse feedback, show the green blob and plus cursor, when a users
alt+drag duplicates a bookmark in the Bookmark manager.

Comment 1

[Mike Pinkerton (not reading bugmail)]

people assume this doesn't work w/out the green plus, we should fix this.

Comment 2

[Geoff "temporarily off bugmail" Beier]

http://lxr.mozilla.org/mozilla/source/camino/src/bookmarks/BookmarkViewController.mm#945

We're only checking to see whether the drag is permitted and returning
NSDragOperationGeneric if it is. While this is technically correct, as it means
the receiver will decide what the operation is when the drop is performed, the
OS will only show the green plus if we actually check the mask there and return
NSDragOperationCopy as appropriate. We should probably 

return (isOK) ? [info draggingSourceOperationMask] : NSDragOperationNone;

instead of

return (isOK) ? NSDragOperationGeneric: NSDragOperationNone;

for types where more than one operation is supported.

Comment 3

[Julian Pellico]

I'll probably look at this

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

*** Bug 183664 has been marked as a duplicate of this bug. ***

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Julian, it seems bug 186114 might be related to this one (in the sense of
"fixing" a part of opt-drag bookmark behavior), although I have no idea if the
code is actually related :-)

Comment 6

[Julian Pellico]

me take

Comment 7

[Julian Pellico]

Attachment: fix drag feedback for mookmarks and tab bar

I did a couple extra small things related to drag-and-drop feedback. Reviewers
please indicate whether you like them.

• The bookmark toolbar will change the cursor to the "green plus" if the user
option-drags from the bookmark manager to indicate that the bookmark will be
copied and not removed from its current location.
• The tab bar will change the cursor to "green plus" when a link or bookmark is
dragged to the tab bar to the right of all the current tabs, to indicate that a
new tab will be created if the mouse is released (there is currently no
indication that anything will happen).

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #7)
> • The tab bar will change the cursor to "green plus" when a link or bookmark is
> dragged to the tab bar to the right of all the current tabs, to indicate that a
> new tab will be created if the mouse is released (there is currently no
> indication that anything will happen).

Not a reviewer, but...this is good IMHO, since the plus is otherwise present and
then disappears in dragging to the tab bar (rf. bug 265994 comment 2).  There's
also some work going on in bug 167689 to improve the dragging-to-tab feedback,
but I think the green plus is expected along with the ghost tab. :)

Comment 9

[Josh Aas]

+  if (dragOpMask == NSDragOperationCopy)
+    return NSDragOperationCopy;
+  if (dragOpMask & NSDragOperationGeneric)
+    return NSDragOperationGeneric;

Why use "==" in the first case, and "&" in the second case? Can multiple bits
ever be set in the mask?

If you explain or standardize your use of those operators in the patch r=joshmoz

Comment 10

[Julian Pellico]

Attachment: same w/comments

updated comments for Josh

Comment 11

[Josh Aas]

Comment on attachment 170801 [details] [diff] [review]
same w/comments

Recent bookmark work made this patch not apply very cleanly. I'd feel more
comfortable if the author redid the patch manually to make sure its still good.
Sorry 'bout that.

If this wasn't the case I would probably have given it r+.

Comment 12

[Josh Aas]

Actually this patch is totally broken now.

Comment 13

[Julian Pellico]

(In reply to comment #12)
> Actually this patch is totally broken now.

yay

Comment 14

[Julian Pellico]

Attachment: no longer broken, for now...

Comment 15

[Josh Aas]

Comment on attachment 172033 [details] [diff] [review]
no longer broken, for now...

Reviewing this bug is irritation so long as 265994 isn't solved yet...

Comment 16

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 172033 [details] [diff] [review]
no longer broken, for now...

sr=pink

Comment 17

[Josh Aas]

landed on trunk