Closed Bug 954822 Opened 10 years ago Closed 10 years ago

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

Categories

(Chat Core :: Twitter, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: benediktp)

Details

Attachments

(2 files)

*** 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.
Attached patch PatchSplinter Review
*** 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: 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.
*** 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+
*** 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.
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
Closed: 10 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.