The 'Copy Link to Tweet' action should be in the context menu even when the twitter account is disconnected

RESOLVED FIXED in 1.4

Status

Chat Core
Twitter
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: florian, Assigned: Mic)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
*** Original post on bio 1387 at 2012-04-23 09:32:00 UTC ***

We used to return early in the getActions method for twitter messages when the account isn't connected because we obviously can't perform any action related to the account in that case, but that's no longer correct now that we have added a 'Copy Link to Tweet' action in the list.
(Assignee)

Comment 1

4 years ago
Created attachment 8354088 [details] [diff] [review]
Patch

*** Original post on bio 1387 as attmnt 2322 at 2013-04-04 19:17:00 UTC ***

The diff looks much worse than the change actually is.
The early return is gone now, the array always initialized and the reply/retweet/(un)follow & delete actions are only pushed into it when the account is connected. Copy link is always added ofcourse.

I tested that the correct actions appear on the context menu when connected and when disconnected (i.e. only "Copy link" here). I also tested that every action still works (except for "retweet" for which I had no worthy tweet in my list;).
Attachment #8354088 - Flags: review?(clokep)
(Assignee)

Updated

4 years ago
Assignee: nobody → benediktp
Status: NEW → ASSIGNED
*** Original post on bio 1387 at 2013-04-04 19:37:14 UTC ***

Comment on attachment 8354088 [details] [diff] [review] (bio-attmnt 2322)
Patch

>diff --git a/chat/protocols/twitter/twitter.js b/chat/protocols/twitter/twitter.js
>+      if (this.incoming) {
>+        actions.push(
>+          new Action(_("action.retweet"), function() {
>+            this.conversation.reTweet(this._tweet);
>+          }, this)
>+        );
>+        let friend = account.isFriend(this._tweet.user);
>+        if (friend !== null) {
Friend will never be null after the changes from bug 955321 (bio 1887).

I'll need to try this / look at it in a better diff viewer.
(Assignee)

Comment 3

4 years ago
*** Original post on bio 1387 at 2013-04-04 21:40:46 UTC ***

(In reply to comment #2)
> Comment on attachment 8354088 [details] [diff] [review] (bio-attmnt 2322) [details]
> Patch
> 
> >diff --git a/chat/protocols/twitter/twitter.js b/chat/protocols/twitter/twitter.js
> >+      if (this.incoming) {
> >+        actions.push(
> >+          new Action(_("action.retweet"), function() {
> >+            this.conversation.reTweet(this._tweet);
> >+          }, this)
> >+        );
> >+        let friend = account.isFriend(this._tweet.user);
> >+        if (friend !== null) {
> Friend will never be null after the changes from bug 955321 (bio 1887).

This patch changed only the indentation on these lines. I'll post a comment on this check in bug 955321 (bio 1887).

> I'll need to try this / look at it in a better diff viewer.

Here's the patch with whitespace changes ignored if that helps: http://pastebin.instantbird.com/168599
Comment on attachment 8354088 [details] [diff] [review]
Patch

*** Original change on bio 1387 attmnt 2322 at 2013-04-05 21:05:23 UTC ***

Looks good. I know you're aware that this might be bitrot soon, so I won't comment on that.
Attachment #8354088 - Flags: review?(clokep) → review+
(Assignee)

Comment 5

4 years ago
Created attachment 8354090 [details] [diff] [review]
Patch, unbitrotted

*** Original post on bio 1387 as attmnt 2324 at 2013-04-05 22:02:00 UTC ***

Unbitrotted patch, carrying forward the r+. This is the patch that should be checked in.
(Assignee)

Updated

4 years ago
Whiteboard: [checkin-needed]
*** Original post on bio 1387 at 2013-04-05 22:24:42 UTC ***

Thanks! http://hg.instantbird.org/instantbird/rev/80b297dbbcf5
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.4
You need to log in before you can comment on or make changes to this bug.