449113 - Bookmark sharing depends on user presence

Status: RESOLVED FIXED

(Cloud Services :: General, defect)

Description

[Anant Narayanan [:anant]]

Currently, new incoming bookmarks are only registered if the user receiving the share is online and logged in to the XMPP server. This behavior should be changed such that users receive notifications of new shares the next time they login.

Comment 1

[Anant Narayanan [:anant]]

Attachment: Make bookmark sharing asynchronous

This patch uses a modified sharing API (which I have pushed to services.mozilla.com), that enables a user to share bookmarks even if the other user is not online.

Notes:

a) The major FIXME is that the user is notified of all bookmarks shared on every sync. This is because I don't know of any way to check if a share has already been accepted or not. This does not cause any functional changes because duplicate folders are NOT created, but merely the annoyance of notifications on every sync.

b) We may want to move into a complete OAuth based solution at some point. Currently, this scheme uses a separate SQLite database on the server to keep track of shares.

Comment 2

[Dan Mills [:thunder]]

Comment on attachment 332271 [details] [diff] [review]
Make bookmark sharing asynchronous


>+  /* FIXME! Gets all shares, not just the new ones. Doesn't impact
>+     functionality because _incomingShareOffer does not create
>+     duplicates, but annoys the user by showing notification of ALL
>+     shares on EVERY sync :(
>+  */

One potential solution here is to keep track locally of shares which are being ignored.  This is similar to what Myk did for tabs ("disposing").

However, this work could be done on a spinoff bug and not this one.

>     /* After syncing the regular bookmark folder contents,
>      * also update both the incoming and outgoing shared folders. */
>     let self = yield;
>+    this._sharing.getNewShares(self.cb);
>+    yield;
>     this.__proto__.__proto__._sync.async(this, self.cb );
>     yield;

New code should use the 'let ret = yield foo();' style as long as it doesn't make the line too wide or otherwise difficult to read.

Otherwise r+ from me.

Comment 3

[Anant Narayanan [:anant]]

http://hg.mozilla.org/labs/weave/index.cgi/rev/e0d77cbf199f