590
edits
Places/AsyncAPIsForSync: Difference between revisions
Jump to navigation
Jump to search
→Notes and feedback on Proposal 1: Responses to mak's feedback.
Comrade693 (talk | contribs) m (→Notes and feedback on Proposal 1: Tag mak's feedback with his name.) |
Comrade693 (talk | contribs) (→Notes and feedback on Proposal 1: Responses to mak's feedback.) |
||
| Line 200: | Line 200: | ||
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. --mak | Name could either be nsIBookmarksService (drop ns?) or IAsyncBookmarks. --mak | ||
Don't have a strong preference. I say just defer to sr on this. --sdwilsh | |||
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 | 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 | ||
I think the whole setup is a bit volatile. We should see what the Sync team says about it and if they care that they might have to do a second query to get the actual name. It's not hard to add it if need be, it just feels awkward. --sdwilsh | |||
nsIAnnotationInfo could most likely drop "flags" support. It has never been really used nor we plan to start using it. --mak | nsIAnnotationInfo could most likely drop "flags" support. It has never been really used nor we plan to start using it. --mak | ||
It's also cheap to keep for now. I think that as long as we support it on nsIAnnotaitonService, we should keep it here. --sdwilsh | |||
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? --mak | |||
I thought about this, but I'm pretty sure that because we store tags as a bookmark in a different root, Sync will just work magically anyway. We should confirm this with them though. I'd rather not add it yet if we can avoid it because this is already a lot of work. --sdwilsh | |||
dynamic containers are a dead-feature-walking. nothing is using them. --mak | dynamic containers are a dead-feature-walking. nothing is using them. --mak | ||
Yeah, we only kinda support them via the type argument. --sdwilsh | |||
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 | 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 | ||
Even if they have a bookmarks observer, they'd have to query to get things like guids (or also check for annotaitons, etc). Using the callback is a lot easier. However, it should be [optional] for all methods that take it. --sdwilsh | |||
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) --mak | 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 | ||
Last modified probably works fine, but we should check with the Sync team. I suspect they might actually care about all cases, in which case, we need to change our approach a bit. --sdwilsh | |||
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 | 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 | ||
I'd be fine with insertItem. --sdwilsh | |||
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? --mak | Is the single instance useless if one can just use the multiple one with a 1-sized array? --mak | ||
Consumer convenience, really. The backend can easily just call the array method (this is how storage does the different executeAsyncs). --sdwilsh | |||
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. --mak | Having to collect info to be able to change them is going to be more expensive than changing them one by one. --mak | ||
How about updateItem then? The implementer should only have to pass in the id or the guid. I envisioned them also being able to specify only the parts that changed: | |||
updateItem({id:2}, {guid:"GUID_HERE"}); | |||
This would set the guid on item with id of 2 to "GUID_HERE". Easy to implement because things will just throw NS_ERROR_NOT_IMPLEMENTED. I hope this doesn't get logged to the error console though... --sdwilsh | |||
regarding the batch thing, same as above, do we need both? --mak | regarding the batch thing, same as above, do we need both? --mak | ||
trivial to do, ends up being easier for the consumer (thinking really of the browser/fennec UI). --sdwilsh | |||
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 | 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 | ||
bookmark annotation. --sdwilsh | |||
=History= | =History= | ||