Closed
Bug 954326
Opened 11 years ago
Closed 11 years ago
Support for twitter entities
Categories
(Chat Core :: Twitter, enhancement)
Chat Core
Twitter
Tracking
(Not tracked)
RESOLVED
FIXED
1.1
People
(Reporter: florian, Assigned: florian)
References
()
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 893 at 2011-07-04 00:01:00 UTC ***
*** Due to BzAPI limitations, the initial description is in comment 1 ***
Assignee | ||
Comment 1•11 years ago
|
||
*** Original post on bio 893 as attmnt 731 at 2011-07-04 00:01:00 UTC ***
I had to touch the OAuth code as it didn't handle the GET parameters yet.
During my limited testing, this code gave the expected results.
It's sad that the search API doesn't return the entities (the streaming API returns them by default and the REST API does it when an additional parameter is provided).
It's also sad that this doesn't resolve all URLs (it only resolves t.co URLs).
The twitter.com website uses an undocumented (private?) https://api.twitter.com/1/urls/resolve.json call to resolve other URLs.
A review would be welcome :).
Attachment #8352473 -
Flags: review?
Comment 2•11 years ago
|
||
Comment on attachment 8352473 [details] [diff] [review]
Patch
*** Original change on bio 893 attmnt 731 at 2011-07-05 16:04:21 UTC ***
I haven't tried this, but it looks good. My only comment is below, and maybe a comment could be put above the parsing of the entities to explain what str, title, text, href, etc. are?
>diff --git a/purple/purplexpcom/src/twitter.js b/purple/purplexpcom/src/twitter.js
>- this.writeMessage(name, aTweet.text, flags);
>+ let text = aTweet.text;
I think I would prefer to see this defined closer to where it is used.
Attachment #8352473 -
Flags: review? → review+
Assignee | ||
Comment 3•11 years ago
|
||
*** Original post on bio 893 as attmnt 733 at 2011-07-05 18:13:00 UTC ***
Same patch but more readable and with an additional comment.
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 8352473 [details] [diff] [review]
Patch
*** Original change on bio 893 attmnt 731 at 2011-07-05 18:13:43 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352473 -
Attachment is obsolete: true
Comment 5•11 years ago
|
||
Comment on attachment 8352475 [details] [diff] [review]
Patch v2
*** Original change on bio 893 attmnt 733 at 2011-07-05 19:17:54 UTC ***
r=me, just tested this and it works well. Thanks for clarifying the parts of each entity!
Attachment #8352475 -
Flags: review+
Assignee | ||
Comment 6•11 years ago
|
||
*** Original post on bio 893 at 2011-07-07 19:59:04 UTC ***
https://hg.instantbird.org/instantbird/rev/605bc38efc13
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.1
Updated•11 years ago
|
Component: General → Twitter
You need to log in
before you can comment on or make changes to this bug.
Description
•