590
edits
Comrade693 (talk | contribs) m (→Notes and feedback on Proposal 1: Tag mak's feedback with his name.) |
|||
| Line 199: | Line 199: | ||
The separate interface is the best bet, both for compatibility and clear sync/async separation. | The separate interface is the best bet, both for compatibility and clear sync/async separation. | ||
Name could either be nsIBookmarksService (drop ns?) or IAsyncBookmarks. | Name could either be nsIBookmarksService (drop ns?) or IAsyncBookmarks. --mak | ||
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. | 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. --mak | ||
nsIAnnotationInfo could most likely drop "flags" support. It has never been really used nor we plan to start using it. | nsIAnnotationInfo could most likely drop "flags" support. It has never been really used nor we plan to start using it. --mak | ||
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? | 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. | dynamic containers are a dead-feature-walking. nothing is using them. --mak | ||
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. | 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. --mak | ||
getBookmarkInfo takes a nsIBookmarkInfo object, thus we probably don't want all of its properties being readonly. | 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) | 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) --mak | ||
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 | 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 --mak | ||
insertBookmarksWithInfo same as above. Fine for batching since it's handled internally. | 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? | Is the single instance useless if one can just use the multiple one with a 1-sized array? --mak | ||
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? | 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. | Having to collect info to be able to change them is going to be more expensive than changing them one by one. --mak | ||
regarding the batch thing, same as above, do we need both? | regarding the batch thing, same as above, do we need both? --mak | ||
Regarding annotations, what should be set? a page or a bookmark annotation? most likely so far we just need bookmark annotations. Page annotations are mostly used for charsets. | Regarding annotations, what should be set? a page or a bookmark annotation? most likely so far we just need bookmark annotations. Page annotations are mostly used for charsets. --mak | ||
=History= | =History= | ||
edits