313740 - Dragging bookmark from Safari to Camino removes bookmark rather than copies

Status: RESOLVED FIXED

(Camino Graveyard :: Bookmarks, defect, P2)

Description

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

Spun off from bug 313338 (reporter never filed a follow-up, but it seemed significant enough):

"Also when transferring individual Bookmarks from Safari to the earlier Camino
version the transfer was a copy leaving the original with Safari. Now the
transfer is total eliminating the Bookmark from Safari."

If you drag a bookmark from Safari's manager to Camino's manager, the bookmark is removed from Safari's manager.  When we go the other way, we copy the bookmark into Safari and keep our original.

In 0.8.4, it was a copy both ways (although moving a bookmark from Safari to Camino resulted in the bookmark's URL being used as its name).  

Any chance this regressed due to changing bookmarks to the shared pasteboard format or whatever?

Comment 1

[Simon Fraser [no longer active]]

Need to use the copy dragging mask for non-local drags.

Comment 2

[Mike Pinkerton (not reading bugmail)]

but these aren't drags we originate. do those flags still matter on the drop site?

Comment 3

[Wevah (Nate Weaver)]

I think it can, if the source implements draggedImage:endedAt:operation: .

Comment 4

[Simon Fraser [no longer active]]

Oh, hrm, misread. Yeah, I'm not sure if we can affect what Safari is doing.

Comment 5

[Nick Kreeger]

Attachment: Forces a call to NSDragOperationCopy if the dropped data is not Camino Bookmarks

Comment 6

[Simon Fraser [no longer active]]

Comment on attachment 200830 [details] [diff] [review]
Forces a call to NSDragOperationCopy if the dropped data is not Camino Bookmarks

This doesn't fix the case of dragging into a root bookmark folder on the left.

Comment 7

[Simon Fraser [no longer active]]

Attachment: Possible alternative patch

Comment 8

[Nick Kreeger]

Comment on attachment 200837 [details] [diff] [review]
Possible alternative patch

This is a better alternative to my patch, looks good, tested and works just fine

Comment 9

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 200837 [details] [diff] [review]
Possible alternative patch

sr=pink 

looks good.

Comment 10

[Simon Fraser [no longer active]]

I don't think I like this patch. It seems that we should be checking for more specific drag operations first, then falling back on NSDragOperationGeneric. It's really safari's problem that it's willing to do a "move" to another application.

Comment 11

[Mike Pinkerton (not reading bugmail)]

but how can we check other ones and fall back if the bug happens because we're doing just that? have we filed a bug with apple that safari is wrong? Doesn't help safari releases already in the field though. 

simon, what do you want to do here?

Comment 12

[Simon Fraser [no longer active]]

Pushing off the 1.0 list.

Comment 13

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

How should we proceed to fix this on our end?

It won't help with all the existing versions of Safari out there (esp. on 10.3.x), but could someone file a bug/talk to Safari team members and get them to fix the bug on their end for the future?

Comment 14

[Chris Lawson (gone)]

I don't suppose there's any way to know where the drag target is if it's not internal to Camino, is there?

If there is, couldn't we simply tell Safari "sorry, we're not moving bookmarks there"?

Comment 15

[Chris Lawson (gone)]

(In reply to comment #14)
> I don't suppose there's any way to know where the drag target is if it's not
> internal to Camino, is there?
> 
> If there is, couldn't we simply tell Safari "sorry, we're not moving bookmarks
> there"?

Ignore the above; I was thinking this bug was backwards of what it really is.

From IRC:

[6:01pm] <Wevah> we should be able to check for dragSourceView == nil or something cos if it's nil, it's from another app

If it's from another app, we should force a copy, rather than allowing the app to be stupid and move things to Camino.

Right?

cl

Comment 16

[froodian (Ian Leue)]

Attachment: Forces drag to NSDragOperationCopy if the dropped data is from an external app

Comment 17

[Wevah (Nate Weaver)]

Comment on attachment 247335 [details] [diff] [review]
Forces drag to NSDragOperationCopy if the dropped data is from an external app

codewise r=me

Comment 18

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 247335 [details] [diff] [review]
Forces drag to NSDragOperationCopy if the dropped data is from an external app

sr=pink

Comment 19

[froodian (Ian Leue)]

Checked in on 1.8branch and trunk.