If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

IRC should notify the user when messages couldn't be sent

RESOLVED FIXED in 1.5

Status

Chat Core
IRC
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Instantbot, Assigned: aleth)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

4 years ago
*** Original post on bio 1554 by scrapmachines AT gmail.com at 2012-06-27 12:44:00 UTC ***

Sometimes, when I try to send a message on IRC chatroom, the message is posted in the conversation window, but instantly, IB displays a line that you are disconnectd, and then it tries to reconnect. After checking I come to know that message is not received by the server, while on the conversation window, it appears like it was sent.

So if a message is not sent in these situations, then the message should remain in the textbox, or come back to the textbox.
*** Original post on bio 1554 at 2012-06-27 12:50:20 UTC ***

It was mentioned on IRC that this happens when there's this error in the error console:
14:10:41 - Optimizer: Error: Connection reset.
14:10:41 - Optimizer: Source File: jar:file:///C:/Program%20Files%20(x86)/Instantbird/omni.ja!/components/irc.js
14:10:41 - Optimizer: Line: 502

I don't think the message should stay in the textbox, but if possible, we should add a system message indicating that the message couldn't be sent.
Summary: If a message is not sent to the IRC server, it should remain in the text box → IRC should notify the user when messages couldn't be sent
(Reporter)

Comment 2

4 years ago
*** Original post on bio 1554 by scrapmachines AT gmail.com at 2012-06-27 12:51:43 UTC ***

Sometimes, its very difficult to type again, what you wanted to send. So it should be displayed somewhere so that one can copy paste it again
*** Original post on bio 1554 at 2012-06-27 12:54:24 UTC ***

(In reply to comment #2)
> Sometimes, its very difficult to type again, what you wanted to send. So it
> should be displayed somewhere so that one can copy paste it again

You just said it's displayed in the conversation, so it's easy to copy/paste.

And you may be interested in bug 954212 (bio 778) (and see the bug 954212 (bio 778) comment 1 :)).
(Assignee)

Comment 4

4 years ago
*** Original post on bio 1554 at 2012-10-17 11:52:00 UTC ***

Does this (comment #1) still happen? Do we have STR?
*** Original post on bio 1554 at 2012-10-17 11:59:49 UTC ***

(In reply to comment #4)
> Does this (comment #1) still happen? Do we have STR?

Yes this is still an issue.

The solution (in my mind) is to ping the server every x minutes (2? 3?), the server should send a pong within a certain number of seconds after that (10? 20?). It's also, we can then reduce the socket timeout since we know data will be being sent every x minutes.
(Reporter)

Comment 6

4 years ago
*** Original post on bio 1554 by scrapmachines AT gmail.com at 2012-10-17 12:01:25 UTC ***

(In reply to comment #4)
> Does this (comment #1) still happen? Do we have STR?

Yes, it still happens.

Whenever my internet is disconnected, and I am in the middle of typing a message, so unknowingly I press enter, and after a second IB tells be that your account is disconnected, but it does not tells me that the last message was sent or not.
(Assignee)

Comment 7

4 years ago
*** Original post on bio 1554 at 2012-10-17 12:13:58 UTC ***

Well then I'm confirming it! ;)
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86 → All
(Assignee)

Comment 8

4 years ago
*** Original post on bio 1554 at 2013-02-10 15:43:18 UTC ***

(In reply to comment #5)
> The solution (in my mind) is to ping the server every x minutes (2? 3?), the
> server should send a pong within a certain number of seconds after that (10?
> 20?). It's also, we can then reduce the socket timeout since we know data will
> be being sent every x minutes.

Since something like this has now been implemented (bug 955245 (bio 1812)), is this still an issue?
(Assignee)

Comment 9

4 years ago
*** Original post on bio 1554 at 2013-02-10 19:48:28 UTC ***

04:52:25 PM - Optimizer: I think the bug is still valid, as even though I get a message saying that "Your account is not connected"
04:52:56 PM - Optimizer: I have no clue whether the message sent just before the "Your account is not connected" was received by the server
04:53:51 PM - Optimizer: although most of the time I take it that the message was not sent and that is the case only
(Assignee)

Comment 10

4 years ago
Created attachment 8353989 [details] [diff] [review]
WIP for testing

*** Original post on bio 1554 as attmnt 2226 at 2013-02-10 19:52:00 UTC ***

Optimizer, could you please test this patch (it's possible to apply the changes inside omni.jar, no need to recompile IB) and see if it helps with the remaining problem, as I can't reproduce.
Attachment #8353989 - Flags: feedback?
(Assignee)

Comment 11

4 years ago
Comment on attachment 8353989 [details] [diff] [review]
WIP for testing

*** Original change on bio 1554 attmnt 2226 at 2013-02-10 20:17:50 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353989 - Attachment description: Patch → WIP for testing
Comment on attachment 8353989 [details] [diff] [review]
WIP for testing

*** Original change on bio 1554 attmnt 2226 at 2013-02-11 02:49:13 UTC ***

I don't particularly like this patch. I think it's a band-aid over the real issue. Unfortunately, I can't really think of a better way to do this...
Attachment #8353989 - Flags: feedback? → feedback+
(Assignee)

Comment 13

4 years ago
Comment on attachment 8353989 [details] [diff] [review]
WIP for testing

*** Original change on bio 1554 attmnt 2226 at 2013-05-30 09:58:58 UTC ***

I agree it's a bandaid, but I ran into this issue yesterday and it's annoying when it happens (I only saw from the log that a couple of messages were never sent). We can always improve the UX in a followup.
Attachment #8353989 - Flags: review?(clokep)
Comment on attachment 8353989 [details] [diff] [review]
WIP for testing

*** Original change on bio 1554 attmnt 2226 at 2013-05-30 10:44:06 UTC ***

To summarize our IRC conversation...

There's on way this will work, aMessage in the caller is just a string, but aMessage in conversationErrorMessage is an incoming message object.
Attachment #8353989 - Flags: review?(clokep) → review-
(Assignee)

Comment 15

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

*** Original post on bio 1554 as attmnt 2464 at 2013-05-30 13:52:00 UTC ***

This only adds an error message when the sent message was a PRIVMSG, as in general we don't know which conversation to display the error message in (the usual problem).
Attachment #8354231 - Flags: review?(clokep)
(Assignee)

Comment 16

4 years ago
Comment on attachment 8353989 [details] [diff] [review]
WIP for testing

*** Original change on bio 1554 attmnt 2226 at 2013-05-30 13:52:51 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353989 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Comment on attachment 8354231 [details] [diff] [review]
Patch

*** Original change on bio 1554 attmnt 2464 at 2013-06-14 10:36:06 UTC ***

I don't love having sendRawMessage return a boolean, but I can't think of a better way to do it. Please add a comment saying that it returns whether it was successful.

Would it make more sense to also bubble this boolean out of sendMessage? This would allow sendMsg (in the conversation) to check the boolean and print directly in the conversation without having to check if it's a PRIVMSG and refind the conversation from aParams[0]. What do you think?
Attachment #8354231 - Flags: review?(clokep) → review-
(Assignee)

Comment 18

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

*** Original post on bio 1554 as attmnt 2484 at 2013-06-14 11:29:00 UTC ***

Yes, that's a better way of doing it.
Attachment #8354251 - Flags: review?(clokep)
(Assignee)

Comment 19

4 years ago
Comment on attachment 8354231 [details] [diff] [review]
Patch

*** Original change on bio 1554 attmnt 2464 at 2013-06-14 11:29:23 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354231 - Attachment is obsolete: true
Comment on attachment 8354251 [details] [diff] [review]
Patch

*** Original change on bio 1554 attmnt 2484 at 2013-06-14 11:57:35 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354251 - Flags: review?(clokep) → review+
(Assignee)

Comment 21

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

*** Original post on bio 1554 as attmnt 2485 at 2013-06-14 12:15:00 UTC ***

Moves the !target.length check to the various commands.
Attachment #8354252 - Flags: review?(clokep)
(Assignee)

Comment 22

4 years ago
Comment on attachment 8354251 [details] [diff] [review]
Patch

*** Original change on bio 1554 attmnt 2484 at 2013-06-14 12:15:13 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354251 - Attachment is obsolete: true
(Assignee)

Comment 23

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

*** Original post on bio 1554 as attmnt 2486 at 2013-06-14 12:23:00 UTC ***

Added comment.
Attachment #8354253 - Flags: review?(clokep)
(Assignee)

Comment 24

4 years ago
Comment on attachment 8354252 [details] [diff] [review]
Patch

*** Original change on bio 1554 attmnt 2485 at 2013-06-14 12:23:33 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354252 - Attachment is obsolete: true
Attachment #8354252 - Flags: review?(clokep)
(Assignee)

Comment 25

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

*** Original post on bio 1554 as attmnt 2487 at 2013-06-14 12:28:00 UTC ***

Adds hyphen.
Attachment #8354254 - Flags: review?(clokep)
(Assignee)

Comment 26

4 years ago
Comment on attachment 8354253 [details] [diff] [review]
Patch

*** Original change on bio 1554 attmnt 2486 at 2013-06-14 12:28:13 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354253 - Attachment is obsolete: true
Attachment #8354253 - Flags: review?(clokep)
Comment on attachment 8354254 [details] [diff] [review]
Patch

*** Original change on bio 1554 attmnt 2487 at 2013-06-14 12:30:31 UTC ***

Looks good, sorry for the long review for this. Let's give this a try in the nightlies.
Attachment #8354254 - Flags: review?(clokep) → review+
Whiteboard: [checkin-needed]
*** Original post on bio 1554 at 2013-06-15 04:11:05 UTC ***

Thanks for fixing this! http://hg.instantbird.org/instantbird/rev/f12655ab11a9
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.5
*** Original post on bio 1554 at 2013-06-17 10:22:16 UTC ***

This is causing xpcshell test failures, any chance you can fix that?

http://buildbot.instantbird.org/builders/win32-nightly-default/builds/977/steps/shell_3/logs/stdio
Created attachment 8354259 [details] [diff] [review]
Fix test bustage

*** Original post on bio 1554 as attmnt 2492 at 2013-06-17 10:34:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354259 - Flags: review?(aleth)
*** Original post on bio 1554 at 2013-06-18 10:17:51 UTC ***

(In reply to comment #23)
> Created attachment 8354259 [details] [diff] [review] (bio-attmnt 2492) [details]
> Fix test bustage

http://hg.instantbird.org/instantbird/rev/83ddae231f8f
(Assignee)

Comment 32

4 years ago
Comment on attachment 8354259 [details] [diff] [review]
Fix test bustage

*** Original change on bio 1554 attmnt 2492 at 2013-06-19 08:05:31 UTC ***

Thanks for fixing this! Sorry I missed it.
Attachment #8354259 - Flags: review?(aleth) → review+
You need to log in before you can comment on or make changes to this bug.