Closed Bug 955084 Opened 10 years ago Closed 10 years ago

Unhandled IRC messages around bans

Categories

(Chat Core :: IRC, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

Details

Attachments

(1 file, 2 obsolete files)

*** Original post on bio 1655 at 2012-08-20 20:15:00 UTC ***

:clokep_work!Instantbir@moz-69FB3955.c3-0.smr-ubr1.sbo-smr.ma.cable.rcn.com MODE #instantbird +b hasOwnProperty!*@*
:concrete.mozilla.org 367 clokep_work #instantbird hasOwnProperty!*@* clokep_work 1345492722
:concrete.mozilla.org 368 clokep_work #instantbird :End of Channel Ban List

The mode one actually causes an error:
Timestamp: 8/20/2012 4:13:23 PM
Error: item is undefined
Source File: chrome://instantbird/content/conversation.xml
Line: 1158

Timestamp: 8/20/2012 4:13:23 PM
Error: Error running command MODE with handler RFC 2812:
{"rawMessage":":clokep_work!Instantbir@moz-69FB3955.c3-0.smr-ubr1.sbo-smr.ma.cable.rcn.com MODE #instantbird +b __proto__!*@*","command":"MODE","params":["#instantbird","+b","__proto__!*@*"],"nickname":"clokep_work","user":"Instantbir","host":"moz-69FB3955.c3-0.smr-ubr1.sbo-smr.ma.cable.rcn.com","source":"Instantbir@moz-69FB3955.c3-0.smr-ubr1.sbo-smr.ma.cable.rcn.com"}
Source File: resource:///modules/ircHandlers.jsm
Line: 101
Source Code:
irc

Timestamp: 8/20/2012 4:13:23 PM
Error: [Exception... "'[JavaScript Error: "item is undefined" {file: "chrome://instantbird/content/conversation.xml" line: 1158}]' when calling method: [nsIObserver::observe]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: resource:///modules/jsProtoHelper.jsm :: <TOP_LEVEL> :: line 413"  data: yes]
Source File: resource:///modules/ircHandlers.jsm
Line: 102
Attached patch WIP v1 (obsolete) — Splinter Review
*** Original post on bio 1655 as attmnt 1879 at 2012-09-01 02:15:00 UTC ***

This kind of handles some of this...but it might be too much...
Attachment #8353637 - Flags: feedback?(bugzilla)
Comment on attachment 8353637 [details] [diff] [review]
WIP v1

*** Original change on bio 1655 attmnt 1879 at 2012-09-01 12:12:20 UTC ***

I don't really see a way in which this could be simpler.
Attachment #8353637 - Flags: feedback?(bugzilla) → feedback+
Attached patch Patch v1 (obsolete) — Splinter Review
*** Original post on bio 1655 as attmnt 1926 at 2012-09-28 02:11:00 UTC ***

This has slightly weird behavior when a new mode is set it will print the new banmask, but then still print out the mode message. I'm not sure what to do about this right now without handling all the possible mode combinations...
Attachment #8353682 - Flags: review?(florian)
Comment on attachment 8353637 [details] [diff] [review]
WIP v1

*** Original change on bio 1655 attmnt 1879 at 2012-09-28 02:11:13 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353637 - Attachment is obsolete: true
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Comment on attachment 8353682 [details] [diff] [review]
Patch v1

*** Original change on bio 1655 attmnt 1926 at 2012-09-28 10:25:18 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353682 - Flags: review?(florian) → review-
*** Original post on bio 1655 at 2012-09-28 10:27:28 UTC ***

Comment on attachment 8353682 [details] [diff] [review] (bio-attmnt 1926)
Patch v1

>diff --git a/chat/locales/en-US/irc.properties b/chat/locales/en-US/irc.properties
>--- a/chat/locales/en-US/irc.properties
>+++ b/chat/locales/en-US/irc.properties
>@@ -119,6 +119,11 @@ message.unknownNick=%S is an unknown nic
> #    channel key (password).
> message.channelKeyAdded=%1$S changed the channel key to %2$S.
> message.channelKeyRemoved=%S removed the channel key.
>+#    This will be followed by a list of ban masks.
>+message.banMasks=The ban masks for %S are:
>+message.noBanMasks=There are no ban masks for %S.
>+message.banMaskAdded=%1$S added the ban mask: %2$S.
>+message.banMaskRemoved=%1$S removed the ban mask: %2$S.

I don't understand what this means, so I don't think users will understand better.

Possible more meaningful string (assuming my guess is right):
Users connected from locations matching %S have been banned by %S.

Note: the 2 strings above talking about "channel key" don't make much sense to me either, but let's pretend I haven't seen them (I've actually never seen them while using the nightlies :)).
*** Original post on bio 1655 at 2012-09-28 15:12:14 UTC ***

(In reply to comment #4)
> >diff --git a/chat/locales/en-US/irc.properties b/chat/locales/en-US/irc.properties
> >@@ -119,6 +119,11 @@ message.unknownNick=%S is an unknown nic
> > #    channel key (password).
> > message.channelKeyAdded=%1$S changed the channel key to %2$S.
> > message.channelKeyRemoved=%S removed the channel key.

> Note: the 2 strings above talking about "channel key" don't make much sense to
> me either, but let's pretend I haven't seen them (I've actually never seen them
> while using the nightlies :)).

They should be changed to say "password" probably, since that's what we call it on the join dialog.

Your strings are better.

aleth, can you still review the code please?
Attached patch Patch v2Splinter Review
*** Original post on bio 1655 as attmnt 1936 at 2012-10-05 23:41:00 UTC ***

Fixed strings.
Attachment #8353692 - Flags: review?(bugzilla)
Comment on attachment 8353682 [details] [diff] [review]
Patch v1

*** Original change on bio 1655 attmnt 1926 at 2012-10-05 23:41:59 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353682 - Attachment is obsolete: true
Comment on attachment 8353692 [details] [diff] [review]
Patch v2

*** Original change on bio 1655 attmnt 1936 at 2012-10-07 16:20:59 UTC ***

I like the new strings!

I agree we should change "channel key" to "channel password", in a separate bug. For now I've added it to the todo list in bug 954855 (bio 1420).
Attachment #8353692 - Flags: review?(bugzilla) → review+
Whiteboard: [checkin-needed]
*** Original post on bio 1655 at 2012-10-07 23:02:24 UTC ***

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