954035 - Twitter support

Status: RESOLVED FIXED

(Chat Core :: Twitter, enhancement)

Description

[Patrick Cloke [:clokep]]

*** Original post on bio 598 at 2010-11-23 17:12:00 UTC ***

There's not bug about Twitter support, but I know flo has worked on it a little bit and I felt the need to post links to some relevant information:
Some code I had worked on briefly: https://hg.instantbird.org/experiments/file/Twitter
Depends on TwitterHelper (from Daniel Glazman): http://sources.disruptive-innovations.com/twitterHelper/tags/latest/TwitterHelper.html
Since that doesn't support OAuth I forked OAuthorizer (from mixedpuppy): http://bitbucket.org/clokep/oauthorizer/ (original: http://bitbucket.org/mixedpuppy/oauthorizer/)
Social Mail includes a version of TwitterHelper compatibly with OAuth: https://addons.mozilla.org/en-US/thunderbird/addon/56120/ http://www.networklighthouse.com/category/development/socialmail/
OAuth-in-JavaScript (made for Firefox extensions -- originally from FireUploader): http://code.google.com/p/oauth-in-javascript/
OAuth: http://oauth.googlecode.com/svn/code/javascript/

Twitter has a bunch of documentation too, this includes a pretty good presentation about the workflow:
http://dev.twitter.com/doc
OAuth authentication: http://dev.twitter.com/pages/auth
JavaScript Libraries: http://dev.twitter.com/pages/libraries#javascript http://dev.twitter.com/pages/oauth_libraries#javascript

There's also the @Anywhere idea, not sure if it's useful to us:
http://dev.twitter.com/anywhere/begin

Comment 1

[Florian Quèze [:florian]]

Attachment: Patch v1 (WIP)

*** Original post on bio 598 as attmnt 452 at 2010-12-26 14:28:00 UTC ***

Feel free to comment!

Comment 2

[Patrick Cloke [:clokep]]

*** Original post on bio 598 at 2010-12-26 15:19:22 UTC ***

Comment on attachment 8352195 [details] [diff] [review] (bio-attmnt 452)
Patch v1 (WIP)

>+  onStatusChange: function(/*in nsIWebProgress*/ aWebProgress,
>+                      /*in nsIRequest*/ aRequest,
>+                      /*in nsresult*/ aStatus,
>+                      /*in wstring*/ aMessage) {
>+  },
Should the user be notified of this, or do we mostly not care?

>--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>+++ b/instantbird/content/browserRequest.xul	Sun Dec 26 15:26:09 2010 +0100
>@@ -0,0 +1,42 @@
>+<?xml version="1.0"?>
>+<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
>+<?xml-stylesheet href="chrome://oauthorizer/skin/oauth.css" type="text/css"?>

>+  <script type="application/x-javascript" src="chrome://instantbird/content/browserRequest.js"/>
>+  <stringbundleset id="stringbundleset">
>+    <stringbundle id="loginBundle"
>+                  src="chrome://oauthorizer/locale/login.properties"/>
>+  </stringbundleset>

These both refer to the oauthorizer extension, is this done on purpose?  They don't seem to be included anywhere below (via makefiles / chrome registration).

Overall the patch looks good, I'm not super familiar with WebProgessListener, but the Twitter account/protocol/chat/conversation look good, and the updates to the XHR object all make sense.

Comment 3

[Florian Quèze [:florian]]

*** Original post on bio 598 at 2010-12-26 16:58:29 UTC ***

(In reply to comment #2)
> (From update of attachment 8352195 [details] [diff] [review] (bio-attmnt 452) [details])
> >+  onStatusChange: function(/*in nsIWebProgress*/ aWebProgress,
> >+                      /*in nsIRequest*/ aRequest,
> >+                      /*in nsresult*/ aStatus,
> >+                      /*in wstring*/ aMessage) {
> >+  },
> Should the user be notified of this, or do we mostly not care?
The latter I think.

> 
> >--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> >+++ b/instantbird/content/browserRequest.xul	Sun Dec 26 15:26:09 2010 +0100
> >@@ -0,0 +1,42 @@
> >+<?xml version="1.0"?>
> >+<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
> >+<?xml-stylesheet href="chrome://oauthorizer/skin/oauth.css" type="text/css"?>
> 
> >+  <script type="application/x-javascript" src="chrome://instantbird/content/browserRequest.js"/>
> >+  <stringbundleset id="stringbundleset">
> >+    <stringbundle id="loginBundle"
> >+                  src="chrome://oauthorizer/locale/login.properties"/>
> >+  </stringbundleset>
> 
> These both refer to the oauthorizer extension, is this done on purpose?  They
> don't seem to be included anywhere below (via makefiles / chrome registration).

The browserRequest.xul window currently looks very ugly, I hope porting some of oauth.css will improve the situation significantly.
I think I've removed all usages of the string bundle, so those 4 lines are most likely just dead code. Thanks for catching it.
I also need to add license headers in these files, and mention the origin.

> Overall the patch looks good, I'm not super familiar with WebProgessListener,
> but the Twitter account/protocol/chat/conversation look good, and the updates
> to the XHR object all make sense.
Great, those are the parts I really worked on!

I still need to add an xAuth code path (and a way to obfuscate the "official secret key").

We also need to make the new tweets appear automatically in the conversation.

And we need to build some UI to make it a good twitter client (but maybe this is another bug). The current patch only opens the home timeline when the account is connected, there's no way to get it back if you close the conversation accidentally. Hmm, maybe I should disconnect the account in the account manager when the conversation is closed, like I do for Omegle.
I'm looking for the minimal feature set we need to be able to release an alpha with it without being ashamed :).

Comment 4

[Patrick Cloke [:clokep]]

*** Original post on bio 598 at 2010-12-26 17:43:25 UTC ***

> We also need to make the new tweets appear automatically in the conversation.
> 
> And we need to build some UI to make it a good twitter client (but maybe this
> is another bug). The current patch only opens the home timeline when the
> account is connected, there's no way to get it back if you close the
> conversation accidentally. Hmm, maybe I should disconnect the account in the
> account manager when the conversation is closed, like I do for Omegle.
> I'm looking for the minimal feature set we need to be able to release an alpha
> with it without being ashamed :).

I'm not sure if the Streaming API could be used (http://dev.twitter.com/pages/streaming_api)?  Otherwise, we can just request the timeline again every xxx seconds starting from the last received response (which I believe is one of the parameters you can send).

Automatically closing might be annoying if someone wants to receive a bunch of tweets, read them...then just get rid of the window until there's new tweets.

Comment 5

[Patrick Cloke [:clokep]]

*** Original post on bio 598 at 2011-01-18 06:03:06 UTC ***

Is this the must up to date patch? What work needs to be done on this? I can probably help to push it along if that's all that is holding back an alpha.

Comment 6

[Florian Quèze [:florian]]

*** Original post on bio 598 at 2011-01-18 10:54:03 UTC ***

(In reply to comment #5)
> Is this the must up to date patch?
I think so.

> What work needs to be done on this?
- Code an xAuth version of the authentication process, and find an offuscated way to store the xAuth-enabled key we have. (probably doesn't hold the alpha)
 
- make the new tweets appear automatically (use of the Steaming API, and detect the duplicates).

- Some very little bit of UI work, to provide a way to reopen the timeline if that conversation has been closed.

Comment 7

[Patrick Cloke [:clokep]]

*** Original post on bio 598 at 2011-01-19 03:22:18 UTC ***

(In reply to comment #6)
> (In reply to comment #5)
> > Is this the must up to date patch?
> I think so.
OK, I'll give it a try tomorrow then.

> > What work needs to be done on this?
> - Code an xAuth version of the authentication process, and find an offuscated
> way to store the xAuth-enabled key we have. (probably doesn't hold the alpha)
I'll ignore this for now.

> - make the new tweets appear automatically (use of the Steaming API, and detect
> the duplicates).
Making them appear shouldn't be difficult, but if I remember. The streaming API pretty much promises nothing about order / duplicates / erased messages. I'm not sure of any good way to do this without a set of all messages in the conversation.

> - Some very little bit of UI work, to provide a way to reopen the timeline if
> that conversation has been closed.
Do we have a proposed UI for this? Some icon (in the status bar?), etc. showing the account is connected which allows reopening the timeline?

Comment 8

[Florian Quèze [:florian]]

*** Original post on bio 598 at 2011-01-19 12:28:42 UTC ***

(In reply to comment #7)

> > - make the new tweets appear automatically (use of the Steaming API, and detect
> > the duplicates).
> Making them appear shouldn't be difficult, but if I remember. The streaming API
> pretty much promises nothing about order / duplicates / erased messages. I'm
> not sure of any good way to do this without a set of all messages in the
> conversation.

About the order, they advise that we display them as they arrive and don't try to reorder them, so we can ignore the issue.

About duplicates, they give no guarantee about it, but from what I've understood of the documentation, they may happen when the server you are connected to changes, so it's very unlikely to have duplicate messages that are more than a few seconds or minutes old.
As each tweet as a unique id, I think it's pretty trivial to keep all the ids of already displayed messages in a JS object and just check if the id of a newly received message is already in the hash to know if we should display or drop it.
Not sure how we would expire the entries of that hash, but for now I guess emptying it completely when the account is disconnected will be enough.

I'm not sure we want to care about erased messages.


Something I was stuck on when trying to use the streaming API is that we need to process the data coming back from an HTTP request as it arrives instead of  when the connection is closed, mostly like you do for IRC. I'm not sure that's possible with the API provided by the XMLHttpRequest object.

> > - Some very little bit of UI work, to provide a way to reopen the timeline if
> > that conversation has been closed.
> Do we have a proposed UI for this?
Unfortunately I don't.

A maybe stupid suggestion: use the join chat dialog. If the chatname is empty, open the home timeline, if the user types something, open a conversation displaying "search result" for that word?
Advantage: no new UI work.
Downside: no current twitter user will think of this without reading it in the release notes.

Comment 9

[Florian Quèze [:florian]]

Attachment: Patch v2

*** Original post on bio 598 as attmnt 505 at 2011-01-27 10:02:00 UTC ***

Changes:
- The streaming API is used to get new messages.
- If the conversation is closed by the user, it's reopened automatically if new messages arrive.
- The user should disconnect the account from the account manager to prevent new messages from being displayed.
- There's a "track" advanced option that allows to specify keywords to track. This is a comma separated list of keywords. Comma means OR, space inside a keyword means AND.

TODO:
- Address the previous comments about the XUL OAuth dialog.
- Handle xAuth (in another bug?).
- Close and reopen the streaming connection when the "track" preference changes. Do we currently have a way to be notified of account specific preference changes?
- Design a UI providing a good user experience (definitely in another bug).

Comment 10

[Florian Quèze [:florian]]

Comment on attachment 8352195 [details] [diff] [review]
Patch v1 (WIP)

*** Original change on bio 598 attmnt 452 at 2011-01-27 10:02:06 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 11

[Florian Quèze [:florian]]

Attachment: interdiff patch (v1->2)

*** Original post on bio 598 as attmnt 506 at 2011-01-27 10:05:00 UTC ***

Interdiff between attachment 8352195 [details] [diff] [review] (bio-attmnt 452) and attachment 8352246 [details] [diff] [review] (bio-attmnt 505).

Comment 12

[Patrick Cloke [:clokep]]

*** Original post on bio 598 at 2011-01-27 16:51:50 UTC ***

(In reply to comment #9)
> Created an attachment (id=505) [details]
> - There's a "track" advanced option that allows to specify keywords to track.
> This is a comma separated list of keywords. Comma means OR, space inside a
> keyword means AND.
We should make sure to note this somewhere (at least in the release notes and probably in the FAQ)

> TODO:
> - Address the previous comments about the XUL OAuth dialog.
Sorry, which previous comment is this?

> - Handle xAuth (in another bug?).
I think we should split this out if we ever want to cut 0.3a1

> - Close and reopen the streaming connection when the "track" preference
> changes. Do we currently have a way to be notified of account specific
> preference changes?
We do not have a way to do this, but it's needed for all JavaScript protocols.

> - Design a UI providing a good user experience (definitely in another bug).
Good luck. :)

Looks good, although browserRequest.js and browserRequest.xul don't have license headers. I'm not sure if you want to add them or not.

Comment 13

[Patrick Cloke [:clokep]]

Comment on attachment 8352246 [details] [diff] [review]
Patch v2

*** Original change on bio 598 attmnt 505 at 2011-01-27 16:52:01 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 14

[Florian Quèze [:florian]]

*** Original post on bio 598 at 2011-01-27 16:55:03 UTC ***

(In reply to comment #11)
> (In reply to comment #9)
> > Created an attachment (id=505) [details] [details]
> > - There's a "track" advanced option that allows to specify keywords to track.
> > This is a comma separated list of keywords. Comma means OR, space inside a
> > keyword means AND.
> We should make sure to note this somewhere (at least in the release notes and
> probably in the FAQ)
> 
> > TODO:
> > - Address the previous comments about the XUL OAuth dialog.
> Sorry, which previous comment is this?

comment 2 and 3 where we discussed the missing licence headers and a few remaining references to oauthizer parts (css, locale files, ...)

Another TODO:
- Localizability!

Comment 15

[Florian Quèze [:florian]]

Attachment: Interdiff patch (v2->final)

*** Original post on bio 598 as attmnt 510 at 2011-01-27 19:18:00 UTC ***

Between attachment 8352246 [details] [diff] [review] (bio-attmnt 505) and the commited version.

Changes:
 - added CSS in the browserRequest dialog.
 - fixed a bug that made "Your account is disconnected." appear way too often.

Comment 16

[Florian Quèze [:florian]]

*** Original post on bio 598 at 2011-01-27 19:20:42 UTC ***

Fixed: https://hg.instantbird.org/instantbird/rev/44e3e5c0c9e2

We will file follow ups for the remaining issues.

Comment 17

[Patrick Cloke [:clokep]]

*** Original post on bio 598 at 2011-01-28 15:15:16 UTC ***

We forgot to package the files: http://hg.instantbird.org/instantbird/rev/dbf0f2b0540b
idechix made some icons for Twitter: http://hg.instantbird.org/instantbird/rev/c6dbd4191421

Comment 18

[Patrick Cloke [:clokep]]

*** Original post on bio 598 at 2011-01-28 20:37:50 UTC ***

This only works for me if I change:
Components.utils.import("resource://app/modules/jsProtoHelper.jsm");
to
Components.utils.import("resource:///modules/jsProtoHelper.jsm");

Otherwise I get an error:
> Error: uncaught exception: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: resource://gre/components/twitter.js :: <TOP_LEVEL> :: line 40"  data: no]

Comment 19

[Florian Quèze [:florian]]

*** Original post on bio 598 at 2011-01-28 23:50:07 UTC ***

Another 3 commits to make this work:
* https://hg.instantbird.org/instantbird/rev/1f7c1b196350 - fix the path to jsProtoHelper.jsm, as proposed in comment 16.

* https://hg.instantbird.org/instantbird/rev/b094da15422d - disable HTML in tweets, so avoid seeng <span> tags around the tweeted message.

* https://hg.instantbird.org/instantbird/rev/1fe884c04f0d - assign conversation unique ids in purpleCoreService instead of purpleConversation + jsProtoHelper to ensure ids are really unique.

Comment 20

[Instantbot]

*** Original post on bio 598 by Michal Stanke <michal.stanke AT mikk.cz> at 2011-08-18 14:20:30 UTC ***

I don't know, if it's applicable, but I found Buzzbird ( http://getbuzzbird.com/bb/ ). Isn't it possible to "use" it's code to improve current Twitter protocol support?