723583 - test pilot thunderbird 'all your user studies' shows each more than once

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )]

Attachment: screenshot showing doubled printing of results.

To reproduce:

1.  studies in ``https://testpilot.mozillalabs.com/testcases/index-tb-dev.json``
2.  tbird with test pilot 1.2:  Preferences > advanced > config editor:  

    extensions.testpilot.indexFileName -> index-tb-dev.json

3.  (clear the cached download - OSX)... 

    $ rm ~/Library/Thunderbird/Profiles/*.default//TestPilotExperimentFiles/index.json

4.  Restart Tbird.
5.  GOOD - doorhanger appears about new study
6.  BAD - ``tools > test pilot > all your user studies > [current]`` 
    (studies show up multiple times)

To investigate...

Starting with ``https://github.com/mozsquib/testpilot.git``, going to look for issues in ``setup.js``

Cf:  https://wiki.mozilla.org/Labs/Test_Pilot/debug

Comment 1

[Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )]

after uninstalling and reinstalling the latest version of the extension (1.3.4), this problem goes away.  @rebron, what version comes with TBird?

Comment 2

[Mark Banner (:standard8)]

1.3.4 currently, but 1.3.5 soon.

I believe I've been seeing this in 1.3.4 though.

Comment 3

[Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )]

Attachment: patch for bug. Neeeds review.

Uses the ``observer`` correctly, rather than the kludgey "assume only one window" hacks that were there before.  

As per Mark Banner:

09:24 <Standard8> updated patch: http://pastebin.mozilla.org/1473807
09:25 <Standard8> the magic is in chrome.manifest
09:25 <Standard8> it sets up TestPilotComponent as a component, and registers it in the profile-after-change category
09:26 <Standard8> when that is going to be called out to the observers, the component gets created as a service, and 
                  then the observer function gets called with profile-after-change
09:26 <Standard8> at which point it registers iteself for mail-startup-done, which is the thing you probably really want 
                  for  Thundebird

Comment 4

[Jim Porter (:squib)]

Comment on attachment 594237 [details] [diff] [review]
patch for bug.  Neeeds review.

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

I wasn't able to reproduce the specific issue in comment 0, but this works for me. We should remove the dump() statements though.

Comment 5

[Mark Banner (:standard8)]

Checked in for Thunderbird:

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

I also bumped the version that comes with Thunderbird so I can get this up on AMO:

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

Moving this to the Thunderbird product so that we can track it is fixed there, if we unify the extensions, then we'll probably want a different method.

Comment 6

[Mark Banner (:standard8)]

Checked into branches:

http://hg.mozilla.org/releases/comm-aurora/rev/3afe7fc35f3
http://hg.mozilla.org/releases/comm-aurora/rev/1f916fceb081
http://hg.mozilla.org/releases/comm-beta/rev/b6fad7040c8e
http://hg.mozilla.org/releases/comm-beta/rev/1257dccef380

Comment 7

[Jim Porter (:squib)]

(In reply to Mark Banner (:standard8) from comment #5)
> http://hg.mozilla.org/comm-central/rev/fc91f140aae6

This still has the debug dump() statements in it. We should remove those.