Last Comment Bug 738810 - convert mailnews/base/prefs/content/AccountManager.js to Services.jsm and MailServices.js (and fix strict Javascript warnings)
: convert mailnews/base/prefs/content/AccountManager.js to Services.jsm and Mai...
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Account Manager (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Thunderbird 15.0
Assigned To: :aceman
:
Mentors:
: 753471 (view as bug list)
Depends on: 224831 709581 739573 740617
Blocks: 720356 720358 755885 80855 749200
  Show dependency treegraph
 
Reported: 2012-03-23 14:35 PDT by :aceman
Modified: 2012-06-02 11:53 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (19.10 KB, patch)
2012-05-12 07:36 PDT, :aceman
iann_bugzilla: review+
Details | Diff | Review
patch v2 (19.84 KB, patch)
2012-05-16 11:55 PDT, :aceman
mconley: review-
Details | Diff | Review
patch v3 (26.33 KB, patch)
2012-05-22 13:52 PDT, :aceman
mconley: review+
Details | Diff | Review
patch v4 (26.34 KB, patch)
2012-05-30 12:53 PDT, :aceman
acelists: review+
Details | Diff | Review

Description :aceman 2012-03-23 14:35:33 PDT
Convert mailnews/base/prefs/content/AccountManager.js to Services.jsm and MailServices.js .

Also update some comments due to landing of IM in
http://hg.mozilla.org/comm-central/rev/8cdfed92867f

Also fix some new strict Javacript warnings:
Timestamp: 23.03.2012 22:23:22
Warning: function onAccountTreeSelect does not always return a value
Source File: chrome://messenger/content/AccountManager.js
Line: 678, Column: 4
Source Code:
    return; 

Timestamp: 23.03.2012 22:23:22
Warning: package is a reserved identifier
Source File: chrome://messenger/content/AccountManager.js
Line: 747, Column: 8
Source Code:
    var package = pageId.split("am-")[1].split(".xul")[0]; 

Timestamp: 23.03.2012 22:23:22
Warning: redefining package is deprecated
Source File: chrome://messenger/content/AccountManager.js
Line: 747, Column: 8
Source Code:
    var package = pageId.split("am-")[1].split(".xul")[0]; 

Timestamp: 23.03.2012 22:23:22
Warning: function getPageFormElements does not always return a value
Source File: chrome://messenger/content/AccountManager.js
Line: 1042

Timestamp: 23.03.2012 22:32:48
Warning: assignment to undeclared variable panel
Source File: chrome://messenger/content/AccountManager.js
Line: 1191

(Depending on bugs that must land first to not make colliding patches.)
Comment 1 :aceman 2012-04-24 13:25:20 PDT
Warning: ReferenceError: reference to undefined property node._account
Source file: chrome://messenger/content/AccountManager.js
Line: 735

When clicking on the SMTP server in the tree.
Comment 2 :aceman 2012-05-09 08:08:00 PDT
To save anybody else the work, I have a patch ready for this, but must wait for bug 739573 to land because they would collide.
Comment 3 :aceman 2012-05-09 12:21:39 PDT
*** Bug 753471 has been marked as a duplicate of this bug. ***
Comment 4 :aceman 2012-05-12 07:36:27 PDT
Created attachment 623424 [details] [diff] [review]
patch
Comment 5 :aceman 2012-05-15 13:54:54 PDT
For reviewers: patch to be applied on top of patch in bug 739573.
Comment 6 Ian Neal 2012-05-15 14:26:21 PDT
Comment on attachment 623424 [details] [diff] [review]
patch

Been running with the patch for a number of days and no issues.
On code inspection...
>+++ b/mailnews/base/prefs/content/am-prefs.js
> function getAccountValueIsLocked(element)
> {
>+  let prefstr = "";
>+  let preftype;
Just define these two within the if statement.
>+  let prefval;
Not used.
> 
>+  let prefstring = element.getAttribute("prefstring");
>   if (prefstring) {
>+    preftype = element.getAttribute("preftype");
>     prefstr = substPrefTokens(prefstring, element);
>     // see if the prefstring is locked
>+    if (prefstr)
>+      return Services.prefs.prefIsLocked(prefstr);
>   }
>   return false;
> }
r=me with those issues addressed
Comment 7 :aceman 2012-05-16 11:55:46 PDT
Created attachment 624475 [details] [diff] [review]
patch v2
Comment 8 Mike Conley (:mconley) - (Needinfo me!) 2012-05-22 13:29:13 PDT
Comment on attachment 624475 [details] [diff] [review]
patch v2

The code looks good, but it looks like it's broken our account Mozmill tests:

SUMMARY-UNEXPECTED-FAIL | test-mail-account-setup-wizard.js | test-mail-account-setup-wizard.js::test_mail_account_setup
  EXCEPTION: amc.window.smtpService is undefined
    at: test-mail-account-setup-wizard.js line 146
       subtest_verify_account test-mail-account-setup-wizard.js 146
       WindowWatcher_notify test-window-helpers.js 365
make: *** [mozmill-one] Error 1


Once you fix that (looks pretty simple), could you push to try to make sure we're all green?
Comment 9 :aceman 2012-05-22 13:52:15 PDT
Created attachment 626167 [details] [diff] [review]
patch v3

Fix test. I can't push to try but I ran that one test myself and it failed before and now it passes.
Comment 10 Mike Conley (:mconley) - (Needinfo me!) 2012-05-28 13:27:36 PDT
I've pushed this patch to try. If they return, I'll give my r+.

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=deb08a2ee9d2
Comment 11 :aceman 2012-05-29 01:47:22 PDT
So there is a green "B X Z" at the line with this ID. Is that good?
Comment 12 Mike Conley (:mconley) - (Needinfo me!) 2012-05-29 06:39:54 PDT
Comment on attachment 626167 [details] [diff] [review]
patch v3

Try build looks OK - the failures that came up are random oranges that we see periodically.
Comment 13 :aceman 2012-05-29 06:47:41 PDT
Great!
Comment 14 :aceman 2012-05-29 07:07:20 PDT
If somebody checking this in reads the comments too thoroughly, then the part of bug 739573 blocking this was already checked in, so this should be free to check-in. The successful try build should confirm that.
Comment 15 :aceman 2012-05-30 12:53:46 PDT
Created attachment 628429 [details] [diff] [review]
patch v4

MPL2 bitrot.
Comment 16 Mike Conley (:mconley) - (Needinfo me!) 2012-06-02 11:53:54 PDT
https://hg.mozilla.org/comm-central/rev/3468a34b21bd

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