Confirmed users
595
edits
Places/AsyncAPIsForSync: Difference between revisions
no edit summary
Comrade693 (talk | contribs) (→Detailed Proposal: Bookmark API propospal) |
No edit summary |
||
| Line 45: | Line 45: | ||
To update already existing items, we call setItemTitle(), changeBookmarkURI(), setKeywordForBookmark() and update various annations. Perhaps there could be an updateBookmarkAsync() akin to insertBookmarkAsync() to save us these various separate method calls? | To update already existing items, we call setItemTitle(), changeBookmarkURI(), setKeywordForBookmark() and update various annations. Perhaps there could be an updateBookmarkAsync() akin to insertBookmarkAsync() to save us these various separate method calls? | ||
==Detailed Proposal== | ==Detailed Proposal 1== | ||
The new methods can be added to a new interface. Maybe nsIBookmarksService (and it only ever does async stuff)? | The new methods can be added to a new interface. Maybe nsIBookmarksService (and it only ever does async stuff)? | ||
| Line 195: | Line 195: | ||
in unsigned long aLength, | in unsigned long aLength, | ||
in nsIBookmarkInfoCallback aCallback); | in nsIBookmarkInfoCallback aCallback); | ||
== Notes and feedback on Proposal 1 == | |||
The separate interface is the best bet, both for compatibility and clear sync/async separation. | |||
Name could either be nsIBookmarksService (drop ns?) or IAsyncBookmarks. | |||
Sync seem to use parentName for reconciliation, but does not look like something that should be part of the API, both the parent and the name can change easily and having 2 volatile informations seem fragile. On the other side caching parent names on the fly in a local hash is probably as efficient. | |||
nsIAnnotationInfo could most likely drop "flags" support. It has never been really used nor we plan to start using it. | |||
nsIBookmarkInfo should most likely include tags or we need a way to set tags async. It's indeed impossible to create a bookmark async and tag it synchronously. it could be possible to add tags in the bookmarks added notification, but is it worth it? | |||
dynamic containers are a dead-feature-walking. nothing is using them. | |||
is nsIBookmarkInfoCallback really useful? Probably yes if the implementer does not want to have a bookmarks observer, that on our side is also expensive. Most likely if the implementer has an observer the callback has no use. | |||
getBookmarkInfo takes a nsIBookmarkInfo object, thus we probably don't want all of its properties being readonly. | |||
Notice that while passing an id or guid will return a single bookmark, passing an uri is not guaranteed to do so. the same uri can have multiple bookmarks. what to do here? either don't allow by uri, or return last modified bookmark (that is what we do today) | |||
insertBookmarkWithInfo looks a strange name. First it is clear I have to pass some info to create a bookmark, second this can create bookmarks, folders, separators... maybe we should go for a generic createItem() method | |||
insertBookmarksWithInfo same as above. Fine for batching since it's handled internally. | |||
Is the single instance useless if one can just use the multiple one with a 1-sized array? | |||
updatebookmarkWithInfo same as above for name. Actually how to handle missing information in the info object? Does the implementer have to collect all info, change and then submit? Or things that should not be changed must be null? How to set a null value then? | |||
Having to collect info to be able to change them is going to be more expensive than changing them one by one. | |||
regarding the batch thing, same as above, do we need both? | |||
=History= | =History= | ||