Closed Bug 954787 Opened 8 years ago Closed 8 years ago

No user feedback when sending a message in a conversation of an offline account

Categories

(Chat Core :: IRC, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: aleth)

References

Details

(Whiteboard: [1.2-blocking][regression])

Attachments

(1 file, 5 obsolete files)

*** Original post on bio 1353 at 2012-03-26 10:43:00 UTC ***

I'm filing this in the IRC component, but it may also happen with other JS-proto accounts.

When sending a message in a conversation (I tried with a nickserv private conversation) while the account is offline, there's no feedback indicating to the user that sending the message failed, and the message is displayed in the conversation like if it was correctly sent!

In the error console I see:

Warning: Failed to convert PRIVMSG NickServ :test from Unicode to UTF-8.
Source File: file:///.../DailyDebug.app/Contents/MacOS/components/irc.js
Line: 772
Source Code:
irc

Error: Socket error: TypeError: this._socket is null
Source File: file:///.../DailyDebug.app/Contents/MacOS/components/irc.js
Line: 775
Source Code:
irc
Whiteboard: [1.2-wanted]
*** Original post on bio 1353 at 2012-04-13 00:17:07 UTC ***

I think this is a regression from bug 953944 (bio 507).
Blocks: 953944
Whiteboard: [1.2-wanted] → [1.2-wanted][regression]
Whiteboard: [1.2-wanted][regression] → [1.2-blocking][regression]
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1353 as attmnt 1438 at 2012-05-04 16:26:00 UTC ***

As discussed. This will work for all accounts.

Should the error messages contain a second sentence with some advice? ("Try setting your status to Available" or "Go to Tools -> Account to try to reconnect" or similar)
Attachment #8353190 - Flags: review?(florian)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1353 as attmnt 1439 at 2012-05-04 16:47:00 UTC ***

Don't break global commands, show error message for protocol-specific ones.
Attachment #8353191 - Flags: review?(florian)
Comment on attachment 8353190 [details] [diff] [review]
Patch

*** Original change on bio 1353 attmnt 1438 at 2012-05-04 16:47:35 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353190 - Attachment is obsolete: true
Attachment #8353190 - Flags: review?(florian)
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1353 as attmnt 1440 at 2012-05-04 16:50:00 UTC ***

Adding an account variable won't improve things as one would have to check if aConversation was defined.
Attachment #8353192 - Flags: review?(florian)
Comment on attachment 8353191 [details] [diff] [review]
Patch

*** Original change on bio 1353 attmnt 1439 at 2012-05-04 16:50:57 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353191 - Attachment is obsolete: true
Attachment #8353191 - Flags: review?(florian)
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1353 as attmnt 1441 at 2012-05-04 17:18:00 UTC ***

Adds a separate message for Services.io.offline == true and fixes some nits.
Attachment #8353193 - Flags: review?(florian)
Comment on attachment 8353192 [details] [diff] [review]
Patch

*** Original change on bio 1353 attmnt 1440 at 2012-05-04 17:18:07 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353192 - Attachment is obsolete: true
Attachment #8353192 - Flags: review?(florian)
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1353 as attmnt 1445 at 2012-05-04 23:08:00 UTC ***

Improve error messages and test for account.statusInfo instead of the global user status.
Attachment #8353197 - Flags: review?(florian)
Comment on attachment 8353193 [details] [diff] [review]
Patch

*** Original change on bio 1353 attmnt 1441 at 2012-05-04 23:08:02 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353193 - Attachment is obsolete: true
Attachment #8353193 - Flags: review?(florian)
Attached patch PatchSplinter Review
*** Original post on bio 1353 as attmnt 1446 at 2012-05-04 23:21:00 UTC ***

Nit fixes
Attachment #8353198 - Flags: review?(florian)
Comment on attachment 8353197 [details] [diff] [review]
Patch

*** Original change on bio 1353 attmnt 1445 at 2012-05-04 23:21:17 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353197 - Attachment is obsolete: true
Attachment #8353197 - Flags: review?(florian)
Comment on attachment 8353198 [details] [diff] [review]
Patch

*** Original change on bio 1353 attmnt 1446 at 2012-05-04 23:23:56 UTC ***

Thanks!
Attachment #8353198 - Flags: review?(florian) → review+
*** Original post on bio 1353 at 2012-05-04 23:36:34 UTC ***

https://hg.instantbird.org/instantbird/rev/97f92dbe9f60
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.