Closed Bug 954463 Opened 6 years ago Closed 4 years ago

Autosetting User Mode in IRC

Categories

(Chat Core :: IRC, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Instantbird 52

People

(Reporter: bugzilla, Assigned: CuriousLearner, Mentored)

Details

(Whiteboard: [good first bug])

Attachments

(1 file, 2 obsolete files)

*** Original post on bio 1028 by J. <sparklingbluestar AT gmail.com> at 2011-09-10 23:21:00 UTC ***

Hello,


I would very much appreciate having an  " Auto-setting for UserMode " 

By that, I mean I would like to be able to automatically set my UserMode to +i, +x, +s when Instantbird starts up ... 

I would also appreciate having an Option to 'Not' have the Accounts Menu automatically Pop-Up when Instantbird starts. 

My concern being I would not want anyone to know with whom I am conversing on Instantbird, if they were to find & access this program.



Thanks in advance for hearing these concerns.



Sincerely,

J.
*** Original post on bio 1028 by J. <sparklingbluestar AT gmail.com> at 2011-09-10 23:28:00 UTC ***

* Append - I meant to say on which Accounts I'm connecting to ... sorry if there was any confusion.
Severity: enhancement → normal
*** Original post on bio 1028 at 2011-10-23 02:31:27 UTC ***

(In reply to comment #0)
> I would very much appreciate having an  " Auto-setting for UserMode " 
> 
> By that, I mean I would like to be able to automatically set my UserMode to +i,
> +x, +s when Instantbird starts up ... 
Yes, this is necessary for some people. I'm confirming this bug.
 
> I would also appreciate having an Option to 'Not' have the Accounts Menu
> automatically Pop-Up when Instantbird starts. 
This is an unrelated problem covered by bug 954480 (bio 1046). If that bug does not cover your issue, please file a separate bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Re: Autosetting UserMode → Autosetting User Mode in IRC
Component: Other → IRC
Product: Instantbird → Chat Core
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: [mentor=clokep][good first bug]
Mentor: clokep
Whiteboard: [mentor=clokep][good first bug] → [good first bug]
Hello Patrick,

I would like to work on this bug. Can you please guide me how to go about it? Thanks.
Hello! We discussed this briefly on IRC and, since it isn't asked for frequently, it would probably make sense as a hidden preference on a per account basis.

What you'd probably want to do is read an option out of the preferences (see [1] for an example), then send it after we've correctly registered with the network (near [2]). Of course, this should only be set if the pref has a value.

If you have not yet built Instantbird directions to do that are available at [3] (or Thunderbird at [4], the same code is used in both). Once you have it working in a fashion you think is reasonable, please export a patch (using |hg diff|) and upload it here and request review of me. Please let me know if you have questions!

[1] http://mxr.mozilla.org/comm-central/source/chat/protocols/irc/irc.js#821
[2] http://mxr.mozilla.org/comm-central/source/chat/protocols/irc/ircBase.jsm#370
[3] https://developer.mozilla.org/en-US/docs/Simple_Instantbird_build
[4] https://developer.mozilla.org/en-US/docs/Simple_Thunderbird_build
Hello,

I've a few questions:

a) I've set up by build but mach run shoots up the nightly version of firefox, how should I go about analysing the Instantbird?
b) What is the IRC channel I can find you/others active on.
c) I was reading the code, how could I know what are the variable the preferences are stored with. I can see we're creating a set of user modes and then picking one up.

Can you please help me a little bit :)
Sanyam,

Sorry this took so long to respond!

a) Please take another look at the "Simple Instantbird Build" page, specifically the section about "Building Instantbird" [1]
b) We're usually in #instantbird on irc.mozilla.org
c) (I'm not fully sure I understand your question, but I'll try to answer anyway.) Preferences can be named whatever we want, in this case I don't think we'll show it in the UI, that means that you'll need to check if the user has it set and then user it if set. I'd likely name it something like autoUserMode.

[1] https://developer.mozilla.org/en-US/docs/Simple_Instantbird_build#Building_Instantbird
Attached patch bug-954463-fix.patch (obsolete) — Splinter Review
Hi, this is what I've done till now. After getting the autoUserMode from preference I'm trying to execute the command to send to the server to set the user mode for the given nick.

Please guide me further.
Attachment #8716919 - Flags: review?(clokep)
Comment on attachment 8716919 [details] [diff] [review]
bug-954463-fix.patch

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

Seems like a good start! I left you a couple of comments.

::: chat/protocols/irc/ircBase.jsm
@@ +370,5 @@
>        this._userModeReceived = false;
>  
> +      // Check if autoUserMode is set in preference
> +      if (this.prefs.prefHasUserValue("autoUserMode")) {
> +        autoUserMode = this.getString("autoUserMode");

You want to declare this variable with `let` in front of it.

@@ +372,5 @@
> +      // Check if autoUserMode is set in preference
> +      if (this.prefs.prefHasUserValue("autoUserMode")) {
> +        autoUserMode = this.getString("autoUserMode");
> +        // Now send it to server to set the mode
> +        this.sendMessage("/mode", this._nickname + " +" + autoUserMode);

This isn't a valid way to change the mode. Take a look at http://tools.ietf.org/html/rfc2812#section-3.1.5
Attachment #8716919 - Flags: review?(clokep) → feedback+
Assignee: nobody → sanyam.khurana01
Status: NEW → ASSIGNED
Hi Patrick,

I'm unable to understand how to proceed further. As far as I've got, sendMessage method is used to send the message to the server, and I was sending the mode command to change the usermode, giving nickname and then setting up the mode.

Could you please elaborate a little bit?
Flags: needinfo?(clokep)
(In reply to Sanyam Khurana [:CuriousLearner] from comment #9)
> I'm unable to understand how to proceed further. As far as I've got,
> sendMessage method is used to send the message to the server, and I was
> sending the mode command to change the usermode, giving nickname and then
> setting up the mode.
"/mode <nickname> +<mode>" is not a valid command to send a server. Please take a look at http://tools.ietf.org/html/rfc2812#section-3.1.5 for the valid way to send this message.
Flags: needinfo?(clokep)
Hi Patrick,

I'm sorry, I'm not able to figure it out from this link, and I tried searching but couldn't get something concrete. Can you provide me with some other link please?
(In reply to Sanyam Khurana [:CuriousLearner] from comment #11)
> I'm sorry, I'm not able to figure it out from this link, and I tried
> searching but couldn't get something concrete. Can you provide me with some
> other link please?

Take a look at the API for sendMessage [1], "/mode" is not a valid command (it's not in RFC 2812, at all). As you can see in section 3.1.5 of that document the command is "MODE", it gives a few examples as:

> MODE WiZ -w
> MODE Angel +i
> MODE WiZ -o

[1] http://mxr.mozilla.org/comm-central/source/chat/protocols/irc/irc.js#1701
Attached patch bug-954463-fix-v2.patch (obsolete) — Splinter Review
Hi,

I've made some changes. Please let me know what needs to be done next. Also, I've a question, how can I check the current user mode for a given user?
Attachment #8741491 - Flags: review?(clokep)
Comment on attachment 8741491 [details] [diff] [review]
bug-954463-fix-v2.patch

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

::: chat/protocols/irc/ircBase.jsm
@@ +372,5 @@
> +      // Check if autoUserMode is set in preference
> +      if (this.prefs.prefHasUserValue("autoUserMode")) {
> +        let autoUserMode = this.getString("autoUserMode");
> +        // Now send it to server to set the mode
> +        this.sendMessage("MODE", this._nickname + " +" + autoUserMode);

The nick and new mode need to be sent as a separate parameters.

Take a look at the API around http://mxr.mozilla.org/comm-central/source/chat/protocols/irc/irc.js#1701, it takes a set of parameters as an array.
Attachment #8741491 - Flags: review?(clokep) → feedback+
Attachment #8716919 - Attachment is obsolete: true
Hi Patrick,

Sorry for getting this in too late. I've updated the patch. Can you please review?
Attachment #8741491 - Attachment is obsolete: true
Flags: needinfo?(clokep)
Attachment #8799305 - Flags: review?(clokep)
No reason to set a review flag and needinfo. I'll look at this soon.
Flags: needinfo?(clokep)
Comment on attachment 8799305 [details] [diff] [review]
bug-954463-fix-v3.patch

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

There's only one more minor change to this! I'd like to get this merged in so I'm going to make it before checking in the change.

Thanks for working on this! Sorry my review took so long.

::: chat/protocols/irc/ircBase.jsm
@@ +358,5 @@
> +      // Check if autoUserMode is set in preference
> +      if (this.prefs.prefHasUserValue("autoUserMode")) {
> +        let autoUserMode = this.getString("autoUserMode");
> +        // Now send it to server to set the mode
> +        this.sendMessage("MODE", [this._nickname, "+" + autoUserMode]);

I think we should let users type in the "+" manually. (You can also "-" at login.)
Attachment #8799305 - Flags: review?(clokep) → review+
https://hg.mozilla.org/comm-central/rev/53570fd871dfa94c7d48a4614cc26faa3e03b508

I think this is your first checkin for comm-central; congrats!
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 52
Nit: We usually have comments as full sentences terminating with a full stop.

// Check if autoUserMode is set in preference
// Now send it to server to set the mode

(I had Patrick complain about a spelling mistake I landed once, so that's why I'm complaining here.)
Thanks, but there was no point to request a full build on a busted tree for this. I took the liberty do cancel all the jobs in the push.
You need to log in before you can comment on or make changes to this bug.