730653 - Support the AppleScript "id" property for bookmark items

Status: RESOLVED FIXED

(Camino Graveyard :: OS Integration, defect)

Description

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

Attachment: Implements "id" backed by UUID

I think Lisa asked me about the standard "id" property of bookmarks about two years ago, and I tried to implement it then and did so poorly, and found for whatever reason (implementor's bug?) that it couldn't actually be used as a reference, so I gave up.

I was thinking about it again recently, and I implemented it again tonight, using my much-improved AppleScript and Cocoa skills.

All* bookmark items now report their UUID when asked for their id, and you can successfully refer to a bookmark item by id and parent collection from then on out, rather than always having to use the full container hierarchy.

  properties of bookmark id "<UUID>" of bookmark bar collection

* There are three classes of exceptions:

1) Separators appear never to be given a UUID; they do end up with a temporary one for the purposes of copying, but it's not persisted.

2) Bookmarks in the two smart collections get UUIDs on creation, but they're not persisted.

3) If the two smart collections and the Top Ten List themselves get UUIDs, they're also not persisted.

So none of those will return an id, since it won't be useful to scripts--and since those are all flat collections, anyway, there's not much benefit to be gained from id+parent collection vs. name+parent collection.  These return nil when asked, which is the 'missing value' keyword in AppleScript parlance, and can be checked for using that keyword.

The code to handle these three classes of exceptions is a little hairy, however.

It's written now with lots of isKindOfClass checks and one cast, but Wevah suggested adding a dummy isSmartFolder to BookmarkItem (apparently there already is a dummy isSeparator on BookmarkItem), which presumably would let me get rid of all of the isKindOfClass checks and the cast?

I also kind of wish we had a method "isReadOnlyCollection" or such that returned Address Book, Bonjour, and Top Ten, rather than having to check isSmartFolder && <some method of determining if we're the Top Ten List>.

So, I'm very open to suggestions on code improvement and the best way to do these things :)

Comment 1

[Stuart Morgan]

Comment on attachment 600749 [details] [diff] [review]
Implements "id" backed by UUID

Top 10 is a smart folder, so the "|| (self == [[BookmarkManager sharedBookmarkManager] top10Folder])" should be redundant.

sr=smorgan with that change.

(I hate the fact that the root bookmark type essentially "knows" about all the subtypes--I think isSeparator is horrible--so I prefer the class checks here.)

Comment 2

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

(In reply to Stuart Morgan from comment #1)
> Top 10 is a smart folder, so the "|| (self == [[BookmarkManager
> sharedBookmarkManager] top10Folder])" should be redundant.
> 
> sr=smorgan with that change.

It's been a while since I worked on this, but I'm certain that from empirical testing I determined that Top 10 is not a smart folder as defined by isSmartFolder, thus this code and my lament at the end of comment 0.  It's special but not smart, IIRC :P

Comment 3

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

Although http://mxr.mozilla.org/camino/source/camino/src/bookmarks/BookmarkManager.mm#418 certainly claims otherwise; I'll double-check.

Comment 4

[Stuart Morgan]

Yeah, that line is what I was basing my claim off of, so my statement is only as trustworthy as that code :)

Comment 5

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

That code in BookmarkManager is right, thankfully :-)

Looking back over my in-progress patches, I believe the redundant bit was a left-over after some much-needed refactoring, and then I remained confused because I had written the comment before the refactoring.

So I removed the redundant bit of code and updated/corrected the comment:
http://hg.mozilla.org/camino/rev/09c69eb1e79c

Comment 6

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

Attachment: sample AppleScript to exercise the functionality

Here's an AppleScript I used for testing that exercises most of the functionality (to use it, you'll have to substitute the right UUIDs for your profile in some cases).

I still need to update the AppleScript guide on the wiki.

Comment 7

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

Attachment: sample AppleScript to exercise the functionality

Grr, actually UTF-16 this time :/

Comment 8

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

(In reply to comment #6)

> I still need to update the AppleScript guide on the wiki.

http://wiki.caminobrowser.org/Development:Camino_AppleScript_Guide#New_in_Camino_2.1.3

Wordsmithery welcome.