Ensure Twitter code will still work after changes to character limit

RESOLVED FIXED in Instantbird 52

Status

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: clokep, Assigned: aleth)

Tracking

trunk
Instantbird 52

Thunderbird Tracking Flags

(thunderbird50 fixed, thunderbird_esr4550+ fixed, thunderbird51 fixed, thunderbird52 fixed)

Details

(Whiteboard: [1.6-blocking])

Attachments

(5 attachments, 4 obsolete attachments)

Reporter

Description

3 years ago
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

3 years ago
Whiteboard: [1.6-blocking]
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

3 years ago
Let's see if this is enough.
Attachment #8793497 - Flags: review?(clokep)
Assignee

Updated

3 years ago
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Assignee

Comment 3

3 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.
> "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 6

3 years ago
Attachment #8793501 - Flags: review?(clokep)
Assignee

Comment 7

3 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

3 years ago
Attachment #8793501 - Flags: review?(clokep) → review+
Reporter

Comment 8

3 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

Updated

3 years ago
Status: ASSIGNED → NEW
Assignee

Comment 11

3 years ago
Posted patch Display extended tweets (obsolete) — Splinter Review
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)
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

3 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? ;)
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

3 years ago
Posted patch Display extended tweets (obsolete) — Splinter Review
Looks like you ran into a RT of an extended tweet.
Attachment #8794622 - Flags: feedback?(clokep)
Assignee

Updated

3 years ago
Attachment #8794597 - Attachment is obsolete: true
Attachment #8794597 - Flags: feedback?(clokep)
Assignee

Comment 16

3 years ago
Posted patch Display extended tweets (obsolete) — Splinter Review
No longer modifies aTweet, slightly improved logging.
Attachment #8794631 - Flags: review?(clokep)
Assignee

Updated

3 years ago
Attachment #8794622 - Attachment is obsolete: true
Attachment #8794622 - Flags: feedback?(clokep)
Assignee

Updated

3 years ago
Attachment #8794631 - Flags: review?(clokep) → review-
Assignee

Comment 17

3 years ago
Posted patch Display extended tweets (obsolete) — Splinter Review
This time modifies the correct if clause... still needs testing though ;)
Attachment #8794633 - Flags: review?(clokep)
Assignee

Updated

3 years ago
Attachment #8794631 - Attachment is obsolete: true
Reporter

Comment 18

3 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

3 years ago
Extra logging removed.
Assignee

Updated

3 years ago
Attachment #8794633 - Attachment is obsolete: true
Assignee

Comment 20

3 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

3 years ago
Attachment #8795520 - Flags: review+
Reporter

Updated

3 years ago
Attachment #8795524 - Flags: review?(clokep) → review+
Assignee

Comment 22

3 years ago
I guess after it's baked on nightlies for a bit we should consider uplifting this to 45?
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 52
Assignee

Updated

3 years ago
Assignee

Comment 23

3 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

3 years ago
Attachment #8793501 - Flags: approval-comm-esr45?
Assignee

Updated

3 years ago
Attachment #8795520 - Flags: approval-comm-esr45?
Assignee

Updated

3 years ago
Attachment #8795524 - Flags: approval-comm-esr45?
Assignee

Comment 24

3 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

3 years ago
Attachment #8793501 - Flags: approval-comm-beta?
Attachment #8793501 - Flags: approval-comm-aurora?
Assignee

Updated

3 years ago
Attachment #8795520 - Flags: approval-comm-beta?
Attachment #8795520 - Flags: approval-comm-aurora?
Assignee

Updated

3 years ago
Attachment #8795524 - Flags: approval-comm-beta?
Attachment #8795524 - Flags: approval-comm-aurora?

Updated

3 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

3 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

3 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

3 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 26

3 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

Updated

3 years ago
Flags: needinfo?(aleth)

Updated

3 years ago
Attachment #8793497 - Flags: approval-comm-esr45? → approval-comm-esr45+

Updated

3 years ago
Attachment #8793501 - Flags: approval-comm-esr45? → approval-comm-esr45+

Updated

3 years ago
Attachment #8795520 - Flags: approval-comm-esr45? → approval-comm-esr45+

Updated

3 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.