Closed
Bug 1275284
Opened 7 years ago
Closed 7 years ago
Ensure Twitter code will still work after changes to character limit
Categories
(Chat Core :: Twitter, defect)
Chat Core
Twitter
Tracking
(thunderbird50 fixed, thunderbird_esr4550+ fixed, thunderbird51 fixed, thunderbird52 fixed)
RESOLVED
FIXED
Instantbird 52
People
(Reporter: clokep, Assigned: aleth)
Details
(Whiteboard: [1.6-blocking])
Attachments
(5 files, 4 obsolete files)
30.32 KB,
patch
|
clokep
:
review+
jorgk-bmo
:
approval-comm-aurora+
jorgk-bmo
:
approval-comm-beta+
rkent
:
approval-comm-esr45+
|
Details | Diff | Splinter Review |
1.41 KB,
patch
|
clokep
:
review+
jorgk-bmo
:
approval-comm-aurora+
jorgk-bmo
:
approval-comm-beta+
rkent
:
approval-comm-esr45+
|
Details | Diff | Splinter Review |
6.12 KB,
text/plain
|
Details | |
4.31 KB,
patch
|
aleth
:
review+
jorgk-bmo
:
approval-comm-aurora+
jorgk-bmo
:
approval-comm-beta+
rkent
:
approval-comm-esr45+
|
Details | Diff | Splinter Review |
2.00 KB,
patch
|
clokep
:
review+
jorgk-bmo
:
approval-comm-aurora+
jorgk-bmo
:
approval-comm-beta+
rkent
:
approval-comm-esr45+
|
Details | Diff | Splinter Review |
Background: https://blog.twitter.com/en-gb/2016/coming-soon-express-even-more-in-140-characters We already request their configuration endpoint (https://dev.twitter.com/rest/reference/get/help/configuration) and use their JavaScript library to get the character count, so I *hope* there's nothing to be done here. Maybe we need to update our Twitter JS library though.
Reporter | ||
Updated•7 years ago
|
Whiteboard: [1.6-blocking]
Comment 1•7 years ago
|
||
Turns out displaying of the longer tweets gets messed up, for example a tweet might appear as "First board game night at @liip in a long time. First game of Citadels ever! With Dutch cards ;) Thanks to… twitter.com/i/web/status/7…" which in its full representation is linked at the end, as it contains an image attachment: https://twitter.com/i/web/status/778660753398198272 The full text reads as "First board game night at @liip in a long time. First game of Citadels ever! With Dutch cards ;) Thanks to @michellesanver @syzer3 and all!"
Assignee | ||
Comment 2•7 years ago
|
||
Let's see if this is enough.
Attachment #8793497 -
Flags: review?(clokep)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•7 years ago
|
||
In that blogpost there's also "Retweet and Quote Tweet yourself: We’ll be enabling the Retweet button on your own Tweets, so you can easily Retweet or Quote Tweet yourself when you want to share a new reflection or feel like a really good one went unnoticed." Is that still correct? If yes, we need another simple patch here.
Comment 4•7 years ago
|
||
> "Retweet and Quote Tweet yourself: We’ll be enabling the Retweet button on your own Tweets, so you can easily Retweet or Quote Tweet yourself when you want to share a new reflection or feel like a really good one went unnoticed."
Yes, you can retweet yourself. You've actually been able to do that since quite some time. The quote retweet shouldn't be a problem, since that's just adding a link to the tweet to the end of the tweet, and you can already copy the link to your own tweets.
Assignee | ||
Comment 5•7 years ago
|
||
https://dev.twitter.com/overview/api/upcoming-changes-to-tweets
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #8793501 -
Flags: review?(clokep)
Assignee | ||
Comment 7•7 years ago
|
||
Hmm, a quick glance at the link in comment 5 makes me wonder if other changes are required as well to display "new-style" tweets correctly.
Reporter | ||
Updated•7 years ago
|
Attachment #8793501 -
Flags: review?(clokep) → review+
Reporter | ||
Comment 8•7 years ago
|
||
Comment on attachment 8793497 [details] [diff] [review] Update twitter-text library to 1.14 Review of attachment 8793497 [details] [diff] [review]: ----------------------------------------------------------------- I'd be surprised if this is enough since what we're talking about is the displaying end, not the tweeting side...but shouldn't hurt!
Attachment #8793497 -
Flags: review?(clokep) → review+
Assignee | ||
Comment 9•7 years ago
|
||
https://hg.mozilla.org/comm-central/rev/7b41f76ddf18b284e860cd741f9c2d02fbea863e Bug 1275284 - Allow self-retweets. r=clokep
Assignee | ||
Comment 10•7 years ago
|
||
https://hg.mozilla.org/comm-central/rev/61b68a1406a6db638d49b8686d904d64985b7072 Bug 1275284 - Update twitter-text library to 1.14. r=clokep
Assignee | ||
Updated•7 years ago
|
Status: ASSIGNED → NEW
Assignee | ||
Updated•7 years ago
|
status-thunderbird52:
--- → affected
tracking-thunderbird52:
--- → ?
Assignee | ||
Comment 11•7 years ago
|
||
I think this should provide basic support, but my test account isn't seeing any extended tweets, so somebody else should test/extend this.
Attachment #8794597 -
Flags: feedback?(clokep)
Comment 12•7 years ago
|
||
I've applied the patch and still see shortened tweets, for example https://twitter.com/i/web/status/780069125934768128 is shortened as it is without the patch to > «Tarzan»-Star #AlexanderSkarsgård kam mit einem Mega-Hangover zum Interview. Wir päppelten ihn mit einer Innerschwe… https://twitter.com/i/web/status/780069125934768128
Assignee | ||
Comment 13•7 years ago
|
||
(In reply to Martin Giger [:freaktechnik] from comment #12) > I've applied the patch and still see shortened tweets, for example > https://twitter.com/i/web/status/780069125934768128 is shortened as it is > without the patch to > > > «Tarzan»-Star #AlexanderSkarsgård kam mit einem Mega-Hangover zum Interview. Wir päppelten ihn mit einer Innerschwe… https://twitter.com/i/web/status/780069125934768128 Could you add the relevant part of the debug log, or fix the problem? ;)
Comment 14•7 years ago
|
||
This tweet is displayed as:
> 21:38:33 - donttrythis: RT @propstore_com: There's ONLY 2 DAYS TO GO until the #PropStoreLiveAuction is finally here!! Register and bid now at… twitter.com/i/web/status/7…
Assignee | ||
Comment 15•7 years ago
|
||
Looks like you ran into a RT of an extended tweet.
Attachment #8794622 -
Flags: feedback?(clokep)
Assignee | ||
Updated•7 years ago
|
Attachment #8794597 -
Attachment is obsolete: true
Attachment #8794597 -
Flags: feedback?(clokep)
Assignee | ||
Comment 16•7 years ago
|
||
No longer modifies aTweet, slightly improved logging.
Attachment #8794631 -
Flags: review?(clokep)
Assignee | ||
Updated•7 years ago
|
Attachment #8794622 -
Attachment is obsolete: true
Attachment #8794622 -
Flags: feedback?(clokep)
Assignee | ||
Updated•7 years ago
|
Attachment #8794631 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 17•7 years ago
|
||
This time modifies the correct if clause... still needs testing though ;)
Attachment #8794633 -
Flags: review?(clokep)
Assignee | ||
Updated•7 years ago
|
Attachment #8794631 -
Attachment is obsolete: true
Reporter | ||
Comment 18•7 years ago
|
||
Comment on attachment 8794633 [details] [diff] [review] Display extended tweets Review of attachment 8794633 [details] [diff] [review]: ----------------------------------------------------------------- I was able to do some minimal testing. The changes look good, but the new debug logs seem a bit verbose. I suggest we remove them before checking this in. What's the newText.trim() change about? Seems unrelated.
Attachment #8794633 -
Flags: review?(clokep) → review+
Assignee | ||
Comment 19•7 years ago
|
||
Extra logging removed.
Assignee | ||
Updated•7 years ago
|
Attachment #8794633 -
Attachment is obsolete: true
Assignee | ||
Comment 20•7 years ago
|
||
This helps debug logs not look like a solid wall of JSON. The newtext.trim() change is to distinguish between actual data and the keepalives that arrive every few seconds. Strictly speaking this would be for another bug, but it's so minor I'm not sure it's worth it.
Attachment #8795524 -
Flags: review?(clokep)
Assignee | ||
Updated•7 years ago
|
Attachment #8795520 -
Flags: review+
Reporter | ||
Updated•7 years ago
|
Attachment #8795524 -
Flags: review?(clokep) → review+
Assignee | ||
Comment 21•7 years ago
|
||
https://hg.mozilla.org/comm-central/rev/8019b72621915032d3477cda86f3b89a98721528 Bug 1275284 - Display extended tweets. r=clokep https://hg.mozilla.org/comm-central/rev/e2d3560b44b0e7465229d8d28956b738c75ee1a9 Bug 1275284 - Make debug log more readable. r=clokep
Assignee | ||
Comment 22•7 years ago
|
||
I guess after it's baked on nightlies for a bit we should consider uplifting this to 45?
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 52
Assignee | ||
Updated•7 years ago
|
tracking-thunderbird_esr45:
--- → ?
Assignee | ||
Comment 23•7 years ago
|
||
Comment on attachment 8793497 [details] [diff] [review] Update twitter-text library to 1.14 [Approval Request Comment] Regression caused by (bug #): Twitter API change User impact if declined: Display of truncated tweets Testing completed (on c-c, etc.): By the time this uplifts, people will have had a chance to report regressions Risk to taking this patch (and alternatives if risky): low
Attachment #8793497 -
Flags: approval-comm-esr45?
Assignee | ||
Updated•7 years ago
|
Attachment #8793501 -
Flags: approval-comm-esr45?
Assignee | ||
Updated•7 years ago
|
Attachment #8795520 -
Flags: approval-comm-esr45?
Assignee | ||
Updated•7 years ago
|
Attachment #8795524 -
Flags: approval-comm-esr45?
Assignee | ||
Comment 24•7 years ago
|
||
Comment on attachment 8793497 [details] [diff] [review] Update twitter-text library to 1.14 [Approval Request Comment] I guess it makes no sense to consider uplifting to 45 without first uplifting to aurora/beta ;)
Attachment #8793497 -
Flags: approval-comm-beta?
Attachment #8793497 -
Flags: approval-comm-aurora?
Assignee | ||
Updated•7 years ago
|
Attachment #8793501 -
Flags: approval-comm-beta?
Attachment #8793501 -
Flags: approval-comm-aurora?
Assignee | ||
Updated•7 years ago
|
Attachment #8795520 -
Flags: approval-comm-beta?
Attachment #8795520 -
Flags: approval-comm-aurora?
Assignee | ||
Updated•7 years ago
|
Attachment #8795524 -
Flags: approval-comm-beta?
Attachment #8795524 -
Flags: approval-comm-aurora?
Updated•7 years ago
|
Attachment #8793497 -
Flags: approval-comm-beta?
Attachment #8793497 -
Flags: approval-comm-beta+
Attachment #8793497 -
Flags: approval-comm-aurora?
Attachment #8793497 -
Flags: approval-comm-aurora+
Updated•7 years ago
|
Attachment #8793501 -
Flags: approval-comm-beta?
Attachment #8793501 -
Flags: approval-comm-beta+
Attachment #8793501 -
Flags: approval-comm-aurora?
Attachment #8793501 -
Flags: approval-comm-aurora+
Updated•7 years ago
|
Attachment #8795520 -
Flags: approval-comm-beta?
Attachment #8795520 -
Flags: approval-comm-beta+
Attachment #8795520 -
Flags: approval-comm-aurora?
Attachment #8795520 -
Flags: approval-comm-aurora+
Updated•7 years ago
|
Attachment #8795524 -
Flags: approval-comm-beta?
Attachment #8795524 -
Flags: approval-comm-beta+
Attachment #8795524 -
Flags: approval-comm-aurora?
Attachment #8795524 -
Flags: approval-comm-aurora+
Comment 25•7 years ago
|
||
Aurora (TB 51): https://hg.mozilla.org/releases/comm-aurora/rev/c08bc7f8009991a4d3041f87a05b24af43d5580d https://hg.mozilla.org/releases/comm-aurora/rev/91784426c81d58787b095eb3475d625a115e7955 https://hg.mozilla.org/releases/comm-aurora/rev/a70dd73f2d13be7abbc4babe139c09a7f8ad6075 https://hg.mozilla.org/releases/comm-aurora/rev/277b70455f1441ccbada9d5bf4dde1c1e61e176b
status-thunderbird50:
--- → affected
status-thunderbird51:
--- → fixed
status-thunderbird_esr45:
--- → affected
tracking-thunderbird52:
? → ---
Comment 26•7 years ago
|
||
Sorry, the second changeset doesn't apply to C-B. There are a heap of changes so I'd rather not try it myself.
Flags: needinfo?(aleth)
Assignee | ||
Comment 27•7 years ago
|
||
https://hg.mozilla.org/releases/comm-beta/rev/cba8e63b8bd77be765549d52ef0a3b0a956b5e37 Bug 1275284 - Allow self-retweets. r=clokep https://hg.mozilla.org/releases/comm-beta/rev/d4134a1be965e2c002454d4341f2a89098c7dc43 Bug 1275284 - Display extended tweets. r=clokep https://hg.mozilla.org/releases/comm-beta/rev/75e7b63716981dfdb764f36dc83fd2fbb28ff3ee Bug 1275284 - Make debug log more readable. r=clokep https://hg.mozilla.org/releases/comm-beta/rev/88269bbc6b03fc3e7defef871713527f5cb96922 Bug 1275284 - Update twitter-text library to 1.14. r=clokep a=jorgk
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(aleth)
Assignee | ||
Updated•7 years ago
|
Comment 28•7 years ago
|
||
https://hg.mozilla.org/releases/comm-esr45/rev/85f5e813a77d https://hg.mozilla.org/releases/comm-esr45/rev/273ed607b04c https://hg.mozilla.org/releases/comm-esr45/rev/75e34d5e927e https://hg.mozilla.org/releases/comm-esr45/rev/d6959430a0aa
Updated•7 years ago
|
Attachment #8793497 -
Flags: approval-comm-esr45? → approval-comm-esr45+
Updated•7 years ago
|
Attachment #8793501 -
Flags: approval-comm-esr45? → approval-comm-esr45+
Updated•7 years ago
|
Attachment #8795520 -
Flags: approval-comm-esr45? → approval-comm-esr45+
Updated•7 years ago
|
Attachment #8795524 -
Flags: approval-comm-esr45? → approval-comm-esr45+
You need to log in
before you can comment on or make changes to this bug.
Description
•