287118 - Change "title and url" bookmark pasteboard format to be compatible with Safari

Status: RESOLVED FIXED

(Camino Graveyard :: Drag & Drop, enhancement)

Description

[Bruce Davidson]

Currently when you drag or copy bookmarks we put the data on the pasteboard in a
variety of formats:
- internal format (@"MozBookmarkType") that gives us UUIDs etc. for the exact
object;
- internal format (@"MozURLType") that gives title and URL for a single bookmark;
- NSURL;
- Plain text (URL only, \n separated for more than one).

Note that despite its name @"MozURLType" is only used by Camino, so we can only
use NSURL and plain text to communicate with Firefox etc. Similarly NSURL or
plain text are the only formats we share with Safari.

Whilst Firefox has no better easily accessible formats, Safari uses a very
similar format WebURLsWithTitlesPboardType that contains an array of [title]
strings and an array or URL strings. We could quite easily convert our code for
MozURLType to support this type instead. We could then drag and drop bookmarks
between Camino and Safari, and any other apps that use the Safari format.

Comment 1

[Bruce Davidson]

Okay, the rough plan for this work is as follows. (All of this after bug 
155484 has been checked in.)

1. Add support for the new format
- Add appropriate constants to NSPasteboard+Utils
- Add utility function to copy a set of URLs and titles to NSPasteboard+Utils
- Add utility function to get an array of URLs from NSPasteboard+Utils. This 
will read the new type if present, or MozURLType/NSURL/NSString (creating an 
array of size 1).
- Where we currently copy items in MozURL format also copy them in the new 
format, adding support for copying multiple items as necessary.
- Add support for pasting the new type into the bookmark view, to give us 
interoperability with Safari.

(Depending on how large the patch is at this point I might seek interim review 
and checkin)

2. Support the new type throughout Camino
- Where we currently accept MozURLType extend to support the new format. This 
includes things like dragging to the URL bar, the tab bar etc. (See, for 
example, bug 230340).

3. Remove MozURLType
- Once step 2 is complete MozURL type will be redundant. It is equivalent to 
the new type with only one URL/title pair in the collection.
- Strip all code that deals with MozURL type.
- Remove support from NSPasteboard+Utils.

I've got some work to do to work out what Safari does with empty titles so 
that we adopt the same convention.

Comment 2

[Bruce Davidson]

Attachment: Outline patch (see comment for limitations)

This is an initial implementation that allows bookmarks to be exchanged between
Camino and Safari using copy and paste (two way) and drag and drop (currently
only from Safari to Camino, as no-one will ever move the other way :-).

I haven't yet set about using the new methods on NSPasteboard+Utils anywhere
else in Camino, though doing so should simplify lots of code. All the code to
support our old "single URL + title" format is still present as the rest of our
code is still generating these.

I hope to continue with more of this work tomorrow. I don't mind whether we
commit the patch on bug 155484 in the interim, or whether we wait for this
"full solution". Any [potential] reviewers with opinions on the easiest way to
get this code reviewed are welcome to comment.

Comment 3

[Bruce Davidson]

Attachment: Full patch

This is a full patch that implements both bug 155484 and the full plan here
(new methods on NSPasteboard+Utils, methods used everywhere we were doing stuff
manually, MozURLType removed).

I haven't attempted to add additional functionality (e.g. dragging multiple
items from history, bug 230430), which this patch should make easy enough to
do.

Comment 4

[Bruce Davidson]

Comment on attachment 179455 [details] [diff] [review]
Full patch

Okay, both Jasper and I seem to get on okay with a build using this patch so
requesting review.

Comment 5

[Simon Fraser [no longer active]]

This does appear to regress dragging a BM toolbar folder into a browser view.

It'a also unfortunate that "Cut" doesn't work, and I think that's a show stopper
for this. Cut is the first thing that I tried to do.

The reason is that all of the existing D&D code assumes that BM items remain in
the BM tree, so the pasteboard can just contain item IDs. With Cut, you have to
put the items themselves in the pasteboard. This requires a bit more work.

Comment 6

[Simon Fraser [no longer active]]

Comment on attachment 179455 [details] [diff] [review]
Full patch

I landed this patch, with some changes that I'll describe over in bug 155484.

Comment 7

[Simon Fraser [no longer active]]

Marking fixed. Let's do tidyup in bug 155484.