Closed
Bug 705808
Opened 13 years ago
Closed 12 years ago
TestPilot first-run tab shouldn't open for Thunderbird
Categories
(Thunderbird :: Testing Infrastructure, defect)
Tracking
(thunderbird9+ fixed, thunderbird10+ fixed)
RESOLVED
FIXED
Thunderbird 11.0
People
(Reporter: mconley, Assigned: mconley)
References
Details
Attachments
(1 file, 1 obsolete file)
12.93 KB,
patch
|
standard8
:
review+
bwinton
:
ui-review+
standard8
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
We noticed this one in bug 700536 - the Account Provisioner modal dialog would block the 3pane, and if the user decided to order an account, the modal would close, the order form tab would open, and then the TestPilot tab would open and steal focus. Our solution for now is to not open the TestPilot tab unless there are one or more accounts associated with the current profile. If not, we hold off on opening it until next time Thunderbird starts with 1 or more accounts.
Assignee | ||
Comment 1•13 years ago
|
||
Here's my first go at it. Am I duplicating too much code? (_appID, the THUNDERBIRD_APP_ID, etc...)
Assignee: nobody → mconley
Attachment #577316 -
Flags: review?(squibblyflabbetydoo)
Comment 2•13 years ago
|
||
Comment on attachment 577316 [details] [diff] [review] Patch v1 Review of attachment 577316 [details] [diff] [review]: ----------------------------------------------------------------- Hmm, you know, I'm sure I submitted this review yesterday. Maybe I dreamt it. Anyway, see my comments below, which were thankfully saved. ::: mail/app/profile/extensions/tbtestpilot@labs.mozilla.com/modules/setup.js @@ +187,5 @@ > + delete this._appID; > + return this._appID = Components.classes["@mozilla.org/xre/app-info;1"] > + .getService(Components.interfaces.nsIXULAppInfo).ID; > + }, > + Since we have this function in two other files already (content/window-utils.js and modules/interface.js), I think it's time we put it somewhere universally accessible to Test Pilot. setup.js might be the best place for this, so you could probably remove the other two instances of it and just call into this. @@ +645,5 @@ > } > return true; > }, > > + _canOpenTabs: function TPS__canOpenTabs() { I think "_shouldOpenTabs" would be more accurate here. @@ +652,5 @@ > + // current profile does not have any accounts associated with it. > + Components.utils.import("resource:///modules/iteratorUtils.jsm"); > + Components.utils.import("resource:///modules/mailServices.js"); > + let numAccounts = [x for each (x in fixIterator(MailServices.accounts.accounts))].length; > + return numAccounts > 0; Can't we say "MailServices.accounts.accounts.Count()"? (Possibly a record for number of times "count" is used on one line!)
Attachment #577316 -
Flags: review?(squibblyflabbetydoo) → review-
Updated•13 years ago
|
tracking-thunderbird10:
--- → +
tracking-thunderbird9:
--- → +
Assignee | ||
Comment 3•13 years ago
|
||
Thanks for the review! I've addressed the issues you brought up, and slightly changed the behaviour of this patch: we now *never* open up the Test Pilot welcome tab, as opposed to waiting until we have 1 or more accounts. Requesting some UI review to make sure this is the right course of action.
Attachment #577316 -
Attachment is obsolete: true
Attachment #579729 -
Flags: ui-review?(bwinton)
Attachment #579729 -
Flags: review?(squibblyflabbetydoo)
Comment 4•12 years ago
|
||
Comment on attachment 579729 [details] [diff] [review] Patch v2 Stealing review
Attachment #579729 -
Flags: review?(squibblyflabbetydoo) → review?(mbanner)
Comment 5•12 years ago
|
||
Comment on attachment 579729 [details] [diff] [review] Patch v2 Yeah, that page seems to contain nothing really useful, and we'll show something similar when the first study pops up, I believe. ui-r=me.
Attachment #579729 -
Flags: ui-review?(bwinton) → ui-review+
Comment 6•12 years ago
|
||
Comment on attachment 579729 [details] [diff] [review] Patch v2 r=me, I'll land this with bitrot fixed when I land bug 709737.
Attachment #579729 -
Flags: review?(mbanner)
Attachment #579729 -
Flags: review+
Attachment #579729 -
Flags: approval-comm-beta+
Attachment #579729 -
Flags: approval-comm-aurora+
Updated•12 years ago
|
Summary: TestPilot first-run tab should not open if profile has no accounts in it. → TestPilot first-run tab shouldn't open for Thunderbird
Comment 7•12 years ago
|
||
Checked in: http://hg.mozilla.org/comm-central/rev/d8c25e08ba23 http://hg.mozilla.org/releases/comm-aurora/rev/0c9973a71788 http://hg.mozilla.org/releases/comm-beta/rev/dd744a951a1d
Status: NEW → RESOLVED
Closed: 12 years ago
status-thunderbird10:
--- → fixed
status-thunderbird9:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 11.0
You need to log in
before you can comment on or make changes to this bug.
Description
•