392213 - nsINavBookmarksService.idl comments need clarification for extension developers

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Dietrich Ayala (:dietrich)]

>  * The comment for insertBookmark claims that "If this item already
> exists in the given folder, it will be moved to the new position." I
> hope this is left over from the earlier implementation and not an
> accurate reflection of how things work today.

> 
>  * getChildFolder claims to return the id of the folder within a
> specified parent folder given a name. I hope this isn't actually used
> anywhere, as it suggests that folder names need to be unique within a
> parent folder. Unless it serves some useful purpose, it should be pulled.

This was used when we used to attempt to merge Fx2 bookmarks imports into Places bookmarks. However, that process was unavoidably imprecise, so we dropped it in favor appending as is done in Fx2. This API is no longer used, and could be dropped.

http://mxr.mozilla.org/seamonkey/search?string=getchildfolder

> 
>  * If the comment for setItemIndex is correct -- namely, that the
> service doesn't take any responsibility for managing the integrity of
> the underlying data -- then I have to say I don't understand the role of
> this API. It's not really helpful to clients if a single inadvertently
> call can corrupt the datasource.

This was required to allow re-sorting from callers external to the service. Yes, we should have bigger warnings here.

We should also clarify the commentary on the get/set read-only apis, to indicate that they're for indicating whether something is read-only in the UI.

Comment 1

[Dietrich Ayala (:dietrich)]

Attachment: update the comments

Comment 2

[Asaf Romano (gone)]

+   *  @returns void

ew!

Comment 3

[Dietrich Ayala (:dietrich)]

(In reply to comment #2)
> +   *  @returns void
> 
> ew!
> 

are you attempting to say that you recommend not putting @returns if void?

Comment 4

[Todd Agulnick]

   /**
    * Set a globally unique identifier.  This can be useful when a sync
    * algorithm deems two independently created items (on different
    * profiles) to be the same item.
    *  @param   aItemId
    *           The id of the item to set the GUID of
-   *  @returns The GUID string
+   *  @returns void
    */
   void setItemGUID(in long long aItemId, in AString aGUID);

What needs correction here is not the description of the return value, but the description of the aGUID arg: the function doesn't return a GUID; it takes one as an argument.

-Todd

Comment 5

[Dietrich Ayala (:dietrich)]

(In reply to comment #4)
>    /**
>     * Set a globally unique identifier.  This can be useful when a sync
>     * algorithm deems two independently created items (on different
>     * profiles) to be the same item.
>     *  @param   aItemId
>     *           The id of the item to set the GUID of
> -   *  @returns The GUID string
> +   *  @returns void
>     */
>    void setItemGUID(in long long aItemId, in AString aGUID);
> 
> What needs correction here is not the description of the return value, but the
> description of the aGUID arg: the function doesn't return a GUID; it takes one
> as an argument.
> 
> -Todd
> 

ack, that'd certainly be the case.

Comment 6

[Dietrich Ayala (:dietrich)]

Attachment: v2

Comment 7

[Asaf Romano (gone)]

@patam aGUID

Comment 8

[Dietrich Ayala (:dietrich)]

Attachment: v3

(In reply to comment #7)
> @patam aGUID
> 

s/t/r/

but i knew what you meant ;)

clearly a banner day.

Comment 9

[Dan Mills [:thunder]]

Comment on attachment 276727 [details] [diff] [review]
v3

Thanks, Dietrich!

Comment 10

[Dietrich Ayala (:dietrich)]

Checking in toolkit/components/places/public/nsINavBookmarksService.idl;
/cvsroot/mozilla/toolkit/components/places/public/nsINavBookmarksService.idl,v  <--  nsINavBookmarksService.idl
new revision: 1.48; previous revision: 1.47
done

Comment 11

[Dietrich Ayala (:dietrich)]

hm, not sure why this is not marked fixed -> FIXED

Comment 12

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv