Closed Bug 1105797 Opened 5 years ago Closed 4 years ago

Unhandled IRC message 501: unknown mode char

Categories

(Chat Core :: IRC, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Instantbird 48

People

(Reporter: aleth, Assigned: mrinal.dhar, Mentored)

Details

(Whiteboard: [good first bug])

Attachments

(1 file, 3 obsolete files)

STR: "/mode +t" in a channel.

WARN. (@ prpl-irc: ircSocket.prototype.onDataReceived resource://gre/components/irc.js:688) 
Unhandled IRC message: 
:fripp.mozilla.org 501 aleth t :is unknown mode char to me
Assignee: nobody → mrinal.dhar
Comment on attachment 8727623 [details] [diff] [review]
Handles the error code by logging relevant message in the server tab

Review of attachment 8727623 [details] [diff] [review]:
-----------------------------------------------------------------

::: chat/locales/en-US/irc.properties
@@ +171,5 @@
>  error.sendMessageFailed=An error occurred while sending your last message. Please try again once the connection has been reestablished.
>  #    %1$S is the channel the user tried to join, %2$S is the channel
>  #    he was forwarded to.
>  error.channelForward=You may not join %1$S, and were automatically redirected to %2$S.
> +error.unknownMode=Unknown value for mode.

How hard would it be to include the mode char in the error message?
"'t' is not a valid user mode on this server."
This should do it.
Attachment #8727623 - Attachment is obsolete: true
Attachment #8727623 - Flags: review?(aleth)
(In reply to Mrinal Dhar from comment #3)
> Created attachment 8727801 [details] [diff] [review]
> Also includes the user mode in the error message
> 
> This should do it.

How?
(In reply to aleth [:aleth] from comment #4)
> (In reply to Mrinal Dhar from comment #3)
> > Created attachment 8727801 [details] [diff] [review]
> > Also includes the user mode in the error message
> > 
> > This should do it.
> 
> How?

I think what aleth is trying to say is that you've added a formatting character (%s) to the format string, but you don't pass in the invalid mode at all to replace that character
Attachment #8727801 - Flags: feedback+
Ah, forgot to run hg qref, my bad. Here's the new patch.
Attachment #8727801 - Attachment is obsolete: true
Attachment #8727837 - Flags: review?
Comment on attachment 8727837 [details] [diff] [review]
Includes the invalid mode in the error message

Review of attachment 8727837 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.

::: chat/locales/en-US/irc.properties
@@ +171,5 @@
>  error.sendMessageFailed=An error occurred while sending your last message. Please try again once the connection has been reestablished.
>  #    %1$S is the channel the user tried to join, %2$S is the channel
>  #    he was forwarded to.
>  error.channelForward=You may not join %1$S, and were automatically redirected to %2$S.
> +error.unknownMode='%S' is not a valid user mode on this server.

Please add a localization note comment so the localisers understand what %S is for.
Attachment #8727837 - Flags: review? → review-
Added Localization note as comment.
Attachment #8727837 - Attachment is obsolete: true
Comment on attachment 8728072 [details] [diff] [review]
Handles error code 501

Review of attachment 8728072 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for taking care of this!
Attachment #8728072 - Flags: review+
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 48
Comment on attachment 8728072 [details] [diff] [review]
Handles error code 501

Review of attachment 8728072 [details] [diff] [review]:
-----------------------------------------------------------------

::: chat/protocols/irc/ircBase.jsm
@@ +1431,5 @@
>        return false;
>      },
>      "501": function(aMessage) { // ERR_UMODEUNKNOWNFLAGS
>        // :Unknown MODE flag
> +      return serverErrorMessage(this, aMessage, _("error.unknownMode", aMessage.params[1]));

Nit: Please try not to exceed the 80 char limit in the future whenever possible.

Fixed on checkin.
You need to log in before you can comment on or make changes to this bug.