We should use .guid and .parentGuid properties, not .guid and .parentGUID or .GUID and .parentGUID.

Marco, please add this bug to the current iteration.

Comment on attachment 8492554 [details] [diff] [review] guids.diff Review of attachment 8492554 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/places/PlacesUIUtils.jsm @@ +354,5 @@ > * The unwrapped data blob of dropped or pasted data. > * @param aType > * The content type of the data. > * @param aNewParentGuid > + * guid of the container the data was dropped or pasted into. I think in comments we should keep it UC when it's a word (not if we are referring to a property) ::: toolkit/components/places/PlacesTransactions.jsm @@ +18,5 @@ > * different. For example, when |undo| is called in order to undo the top undo > * entry, the caller cannot tell for sure what entry would it be because the > * execution of some transaction is either in process, or queued. > * > + * guids and item-ids ditto @@ +23,4 @@ > * ------------------- > * The Bookmarks API still relies heavily on item-ids, but since those do not > * play nicely with the concept of undo and redo (especially not in an > + * asynchronous environment), this API only accepts bookmark guids, both for ditto @@ +27,2 @@ > * input (e.g. for specifying the parent folder for a new bookmark) and for > + * output (when the guid for such a bookmark is propagated). ditto @@ +29,4 @@ > * > + * guids are readily available when dealing with the "output" of this API and > + * when result nodes are used (see nsINavHistoryResultNode::bookmarkGuid). > + * If you only have item-ids in hand, use PlacesUtils.promiseItemGuid for ditto @@ +45,5 @@ > * property for NewBookmark). Once a transaction is created, you may pass it > * to |transact| or use it in the for batching (see next section). > * > * The constructors throw right away when any required input is missing or when > + * some input is invalid "on the surface" (e.g. guid values are validated to be ditto @@ +59,5 @@ > * - siteURI: an nsIURI object, holding the url for the site with which > * a live bookmark is associated. > * - tag - a string. > * - tags: an array of strings. > + * - guid, parentGuid, newParentGuid: a valid places guid string. ditto (only the last one) @@ +77,5 @@ > * by numerous NewBookmark transactions - all to be undone or redone in a single > * command. The |transact| method makes this possible using a generator > * function as an input. These generators have the same semantics as in > * Task.jsm except that when you yield a transaction, it's executed, and the > + * resolution (e.g. the new bookmark guid) is sent to the generator so you can ditto @@ +774,3 @@ > * The function to be called for creating the item on execute and redo. > * It should return the itemId for the new item > + * - aGuidToRestore - the guid to set for the item (used for redo). ditto @@ +938,3 @@ > * Optional Input Properties: index, title, keyword, annotations, tags. > * > + * When this transaction is executed, it's resolved to the new bookmark's guid. ditto @@ +977,3 @@ > * Optional Input Properties: index, annotations. > * > + * When this transaction is executed, it's resolved to the new folder's guid. ditto @@ +1000,4 @@ > * Optional Input Properties: index. > * > * When this transaction is executed, it's resolved to the new separator's > + * guid. ditto @@ +1022,4 @@ > * Optional Input Properties: siteURI, index, annotations. > * > * When this transaction is executed, it's resolved to the new separators's > + * guid. ditto ::: toolkit/components/places/PlacesUtils.jsm @@ +1475,5 @@ > /** > * Promised wrapper for mozIAsyncHistory::getPlacesInfo for a single place. > * > * @param aPlaceIdentifier > + * either an nsIURI or a guid (@see getPlacesInfo) ditto @@ +1554,5 @@ > * > * @param aItemId > * an item id > * @return {Promise} > + * @resolves to the guid. ditto @@ +1564,5 @@ > * Get the item id for an item (a bookmark, a folder or a separator) given > * its unique id. > * > + * @param aGuid > + * an item guid ditto @@ +1569,3 @@ > * @retrun {Promise} > + * @resolves to the guid. > + * @rejects if there's no item for the given guid. ditto @@ +1600,5 @@ > * > * @return {Promise} > * @resolves to a JS object that represents either a single item or a > * bookmarks tree. Each node in the tree has the following properties set: > + * - guid (string): the item's guid (same as aItemGuid for the top item). ditto (only the middle one) @@ +1618,2 @@ > * properties set: > + * - parentGuid (string): the guid of the root's parent. This isn't set if ditto (the last one) @@ +1920,4 @@ > // The current bookmarks API, however, doesn't expose the necessary means for > +// working with guids. So, until it does, this helper object accesses the > +// Places database directly in order to switch between guids and itemIds, and > +// "restore" guids on items re-created items. all of the comment block above @@ +1967,5 @@ > /** > * This observers serves two purposes: > * (1) Invalidate cached id<->guid paris on when items are removed. > + * (2) Cache Guids given us free of charge by onItemAdded/onItemRemoved. > + * So, for exmaple, when the NewBookmark needs the new guid, we already ditto ::: toolkit/components/places/nsINavBookmarksService.idl @@ +276,5 @@ > * The index to insert at, or DEFAULT_INDEX to append > * @param aTitle > * The title for the new bookmark > + * @param [optional] aGuid > + * The guid to be set for the new item. If not set, a new guid is ditto @@ +305,5 @@ > * The name of the new folder > * @param aIndex > * The index to insert at, or DEFAULT_INDEX to append > + * @param [optional] aGuid > + * The guid to be set for the new item. If not set, a new guid is ditto @@ +364,5 @@ > * The id of the parent folder > * @param aIndex > * The separator's index under folder, or DEFAULT_INDEX to append > + * @param [optional] aGuid > + * The guid to be set for the new item. If not set, a new guid is ditto