Last Comment Bug 735347 - Prefing off the IM feature should also hide the "Chat" preference pane
: Prefing off the IM feature should also hide the "Chat" preference pane
Status: RESOLVED FIXED
[esr]
:
Product: Thunderbird
Classification: Client Software
Component: Instant Messaging (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 14.0
Assigned To: Florian Quèze [:florian] [:flo]
:
:
Mentors:
Depends on:
Blocks: tb-enterprise
  Show dependency treegraph
 
Reported: 2012-03-13 11:24 PDT by Florian Quèze [:florian] [:flo]
Modified: 2012-04-02 04:09 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
Patch (3.02 KB, patch)
2012-03-21 15:18 PDT, Florian Quèze [:florian] [:flo]
bwinton: review+
mozilla: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description Florian Quèze [:florian] [:flo] 2012-03-13 11:24:06 PDT
The "mail.chat.enabled" pref currently prevents the IM code from being initialized, hides the IM UI from the main window ("Chat" button in the toolbar; menu items) and from the Account Settings dialog (hides IM accounts from the list and hides the "Add Chat account…" menu item), but the "Chat" prefpane is still visible in the pref window.
Comment 1 Florian Quèze [:florian] [:flo] 2012-03-21 15:18:51 PDT
Created attachment 608112 [details] [diff] [review]
Patch

I wish I could just set .hidden = true on the prefpane, but the preference bindings don't like it that way...

Also, I'm not very sure of what the story is with Services.jsm in the prefwindow with all these dynamically loaded overlay files, so I played safe and used the old Components.classes[...].getService form to get the pref service.
Comment 2 Blake Winton (:bwinton) (:☕️) 2012-03-27 13:05:20 PDT
Comment on attachment 608112 [details] [diff] [review]
Patch

>+++ b/mail/components/preferences/jar.mn
>@@ -1,15 +1,17 @@
> messenger.jar:
>+    content/messenger/preferences/preferences.js

So, "preferences.js" seems really generic.  Is there another name we could choose that more accurately describes what it does?
(It probably doesn't matter, so I'm not going to block the review over it, but if you can think of a better name, I'ld appreciate it.  :)

>+    content/messenger/preferences/chat.js

Why are you adding chat.js here?

>+++ b/mail/components/preferences/preferences.js
>@@ -0,0 +1,16 @@
>+/* This Source Code Form is subject to the terms of the Mozilla Public
>+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>+ * You can obtain one at http://mozilla.org/MPL/2.0/. */

Love that MPL2!  :)

I mostly like it.  r=me with the chat.js removed, or an explanation of why it's necessary (and wasn't necessary before).

Thanks,
Blake.
Comment 3 Florian Quèze [:florian] [:flo] 2012-03-27 13:52:09 PDT
(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #2)
> Comment on attachment 608112 [details] [diff] [review]
> Patch
> 
> >+++ b/mail/components/preferences/jar.mn
> >@@ -1,15 +1,17 @@
> > messenger.jar:
> >+    content/messenger/preferences/preferences.js
> 
> So, "preferences.js" seems really generic.  Is there another name we could
> choose that more accurately describes what it does?

For me "preferences.js" in this context means "the JS file loaded with the XUL file named preferences.xul" so I can't think of a better name.
I was quite surprised that this file didn't exist yet.

> >+    content/messenger/preferences/chat.js
> 
> Why are you adding chat.js here?

It's an unrelated change, I pondered filing a separate bug and attaching a separate patch for it, but that seemed like too much clicking.

chat.xul includes chat.js that contains code to disable the subitems of the "Let my contacts know that I'm idle" when that checkbox is unchecked. However that didn't work because I forgot to package chat.js...
Comment 4 Florian Quèze [:florian] [:flo] 2012-03-28 02:45:17 PDT
http://hg.mozilla.org/comm-central/rev/41aa34975063
Comment 5 Florian Quèze [:florian] [:flo] 2012-03-30 05:56:14 PDT
Comment on attachment 608112 [details] [diff] [review]
Patch

[Approval Request Comment]
This is required if we want to have the ability to pref off the IM feature.
Comment 6 Florian Quèze [:florian] [:flo] 2012-04-02 04:09:36 PDT
http://hg.mozilla.org/releases/comm-aurora/rev/7fdb23d4f87b

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