Closed Bug 679513 Opened 13 years ago Closed 13 years ago

[Tracking] Add Test Pilot to Thunderbird

Categories

(Thunderbird :: General, defect)

8 Branch
x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 9.0

People

(Reporter: rebron, Assigned: squib)

References

Details

Attachments

(2 files, 2 obsolete files)

Add Test Pilot to Thunderbird so we can offer usability tests to our users and make Thunderbird better.
Attached patch WIP patch (obsolete) — Splinter Review
Here's a WIP version of the Test Pilot add-on imported into Thunderbird. This is an exact copy of the standalone version on Bitbucket <https://bitbucket.org/squib/tb-testpilot>, which means that any bugs in that version are probably present here (notably, the All Studies window on Mac is transparent for some reason, and we point to the wrong index file).

The add-on should install itself into a new profile, but not an existing one. I believe this is what happens in Firefox too, but maybe we want to figure out how to install it in existing profiles.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
Attachment #561972 - Flags: feedback?(mbanner)
Comment on attachment 561972 [details] [diff] [review]
WIP patch

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

Minor couple of comments, I still need to run with this, which I'll try and do first thing tomorrow.

::: mail/app/Makefile.in
@@ +68,4 @@
>  DEFINES += -DOFFICIAL_BUILD=1
>  endif
>  
> +DIRS		= profile profile/extensions

I didn't realise we had a makefile in profile, so might as well just call extensions from the one in profile for now.

::: mail/app/profile/extensions/Makefile.in
@@ +45,5 @@
> +include $(DEPTH)/config/autoconf.mk
> +
> +include $(topsrcdir)/config/rules.mk
> +
> +#ifneq (,$(filter aurora beta,$(MOZ_UPDATE_CHANNEL)))

So for now, I'd like this on nightly, aurora & beta, we can think about release when we get a bit closer.
Couple more notes:

I'm not convinced the welcome to test pilot on a fresh profile is what we really want, that's going to confuse users. We can deal with that in a follow-up.

We need to do some s/browser/mail/ in the initial tab as well (testpilot.welcomePage.iconExplanation in main/properties).

For the issue with not installing add-ons in existing profiles, I'm wondering if we've not go some code that FF has. I think we can let the code land as-is before Tuesday and fix that later.

Re the study status page, it looks like we're displaying the mobile one and not the normal one, could that be the issue? If you could track down where it is called, that might help.
(In reply to Mark Banner (:standard8) from comment #3)
> Re the study status page, it looks like we're displaying the mobile one and
> not the normal one, could that be the issue? If you could track down where
> it is called, that might help.

At least on my build, it seems we're calling the normal all studies window (I'm opening it from Tools -> Test Pilot).
Attached patch Mostly-complete patch (obsolete) — Splinter Review
Aside from stuff we can handle in followups, I think this is finished.
Attachment #561972 - Attachment is obsolete: true
Attachment #561972 - Flags: feedback?(mbanner)
Attachment #562267 - Flags: review?(mbanner)
I think this is basically ready to land, with a couple of minor tweaks:

- Change the id to tbtestpilot@labs.mozilla.org so that we don't clash with the Firefox version.

- Confirm that index-tb.json is the release version.

I'm hoping to get some time tomorrow morning to look at the transparency issue - I was only seeing it on the individual studies page and not the all studies.

If not, then we'll land with that and try and fix it as soon as we can.
Attached patch Update addon IDSplinter Review
This version changes the add-on's ID to tbtestpilot@labs.mozilla.com and the name of the add-on to "Test Pilot for Thunderbird".
Attachment #562267 - Attachment is obsolete: true
Attachment #562267 - Flags: review?(mbanner)
Attachment #562656 - Flags: review?(mbanner)
Attachment #562656 - Flags: review?(mbanner) → review+
I had to add a DEFINES statement to the extensions/Makefile.in for the Thunderbird Version, not sure why I didn't pick that up before:

http://hg.mozilla.org/comm-central/rev/bfdcd57f0ad1

Re test pilot installation - it looks at changes in the distribution directory when the version of Thunderbird changes, hence landing it now is good because the version will change tomorrow and everyone will pick it up.

We still need to file a few bugs on the follow-ups though.
Target Milestone: --- → Thunderbird 9.0
So somehow in the review of the previous patch I totally missed the fact that Thunderbird wasn't building this when the channel was set, and it wasn't actually going to be packaged either.

This patch resolves that.

- Adding MOZ_UPDATE_CHANNEL to configure.in and autoconf.mk.in means that the code in mail/app/profile/extensions/Makefile.in actually gets the definition it needs (this is the same way as FF does it).
- Adding the /installer/ code again reflects FF (http://mxr.mozilla.org/comm-central/search?string=SHIP_FEEDBACK) and means we actually package test pilot.
Attachment #566278 - Flags: review?(gozer)
Attachment #566278 - Flags: review?(dbienvenu)
Comment on attachment 566278 [details] [diff] [review]
Actually build and distribute it

I'm still building this, but I'm going to give a conditional r+ since it looks OK...
Attachment #566278 - Flags: review?(dbienvenu) → review+
Attachment #566278 - Flags: review?(gozer) → review+
Checked into central.
(In reply to Mark Banner (:standard8) from comment #11)
> Checked into central.

http://hg.mozilla.org/comm-central/rev/99b3ebba53e4
I also had to do a unit test bustage fix to stop the test pilot tab coming up:

http://hg.mozilla.org/comm-central/rev/57183fd2f075

(Mozmill sees test pilot as a fresh install).

Lets call this fixed, and we'll deal with other issues in follow-up bugs.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.