Closed Bug 954881 Opened 10 years ago Closed 10 years ago

Stop requesting WHOIS information when offline

Categories

(Chat Core :: IRC, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

Details

Attachments

(1 file, 3 obsolete files)

*** Original post on bio 1447 at 2012-05-19 15:48:00 UTC ***

Currently if you mouseover an IRC buddy (on your contact list) while your account is disconnected, it attempts to send the WHOIS info string.

I assume something similar will happen for a conversation of a disconnected account.
*** Original post on bio 1447 at 2012-05-19 23:29:00 UTC ***

Also happens for nicks in the participant list of chats.
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1447 as attmnt 1490 at 2012-05-19 23:36:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353243 - Flags: review?(clokep)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
*** Original post on bio 1447 at 2012-05-20 15:20:07 UTC ***

(In reply to comment #2)
> Created attachment 8353243 [details] [diff] [review] (bio-attmnt 1490) [details]
> Patch

I think that this will work OK (but I would want florian's opinion on this as well). I do wonder if this would be hiding another issue though (we seem to be attempting to send IRC commands when offline == bad).
*** Original post on bio 1447 at 2012-05-20 16:50:07 UTC ***

The implicit question I had for the reviewer was whether this check should instead be made in the IRC requestBuddyInfo method (rather than in buddytooltip.xml). This depends on how the other protocols handle it when offline I suppose. (Twitter seems to have no problems with it when disconnected, but I am not sure that generalizes.)
Attached patch Alternative patch (obsolete) — Splinter Review
*** Original post on bio 1447 as attmnt 1506 at 2012-05-22 00:19:00 UTC ***

I think I would prefer a patch like this one as maybe some accounts could actually request information out of band from really being "connected"? Opinions?
Attachment #8353258 - Flags: feedback?
Comment on attachment 8353258 [details] [diff] [review]
Alternative patch

*** Original change on bio 1447 attmnt 1506 at 2012-05-22 00:24:38 UTC ***

I'm not sure this will do in the present form, at least not on it's own, because requestBuddyInfo in buddytooltip.xml will add an observer that will never receive a response. I'm not certain this will cause problems, but it needs checking.
Attachment #8353258 - Flags: feedback? → feedback-
Attached patch Alternative patch v2 (obsolete) — Splinter Review
*** Original post on bio 1447 as attmnt 1510 at 2012-05-22 21:46:00 UTC ***

I'm stealing this from you. I don't think blocking the call in the UI is the right way to fix this.
Attachment #8353262 - Flags: review?(bugzilla)
Comment on attachment 8353243 [details] [diff] [review]
Patch

*** Original change on bio 1447 attmnt 1490 at 2012-05-22 21:46:39 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353243 - Attachment is obsolete: true
Attachment #8353243 - Flags: review?(clokep)
Comment on attachment 8353258 [details] [diff] [review]
Alternative patch

*** Original change on bio 1447 attmnt 1506 at 2012-05-22 21:46:39 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353258 - Attachment is obsolete: true
Assignee: aleth → clokep
Comment on attachment 8353262 [details] [diff] [review]
Alternative patch v2

*** Original change on bio 1447 attmnt 1510 at 2012-05-22 21:51:30 UTC ***

With second thoughts, I think this is better as well. Thanks!
Attachment #8353262 - Flags: review?(bugzilla) → review+
Whiteboard: [checkin-needed]
*** Original post on bio 1447 at 2012-05-22 22:15:51 UTC ***

Comment on attachment 8353262 [details] [diff] [review] (bio-attmnt 1510)
Alternative patch v2

># HG changeset patch
># User Patrick Cloke <clokep@gmail.com>
># Date 1337723081 14400
># Node ID 922d518a187623c2eddd4f2e67c859210312ccd0
># Parent  e233c3ee061b8155de4e76a8dc73274031abd0ee
>Bug 954881 (bio 1447) - Stop requesting WHOIS information when offline.
>
>diff --git a/chat/protocols/irc/irc.js b/chat/protocols/irc/irc.js
>--- a/chat/protocols/irc/irc.js
>+++ b/chat/protocols/irc/irc.js
>@@ -641,6 +641,12 @@ ircAccount.prototype = {
>   // Request WHOIS information on a buddy when the user requests more
>   // information.
>   requestBuddyInfo: function(aBuddyName) {
>+    if (!this.connected) {
>+      Services.obs.notifyObservers(EmptyEnumerator,
>+                                   "user-info-received", aBuddyName);
>+      return;
>+    }
>+

If you want to match the behavior we have for libpurple accounts, you should just return early without notifying the observer (which info have you received that you want to notify the UI about anyway?).

See http://lxr.instantbird.org/instantbird/source/purple/purplexpcom/src/purpleAccount.cpp#619 for the purplexpcom implementation.
*** Original post on bio 1447 as attmnt 1511 at 2012-05-22 22:19:00 UTC ***

Matches the libpurple behavior
Attachment #8353263 - Flags: review?(florian)
Comment on attachment 8353262 [details] [diff] [review]
Alternative patch v2

*** Original change on bio 1447 attmnt 1510 at 2012-05-22 22:19:24 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353262 - Attachment is obsolete: true
Comment on attachment 8353263 [details] [diff] [review]
Alternative patch v3

*** Original change on bio 1447 attmnt 1511 at 2012-05-22 22:23:58 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353263 - Flags: review?(florian) → review+
*** Original post on bio 1447 at 2012-05-22 23:09:06 UTC ***

Checked in as http://hg.instantbird.org/instantbird/rev/d43ef47d5e8c
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.