Last Comment Bug 723583 - test pilot thunderbird 'all your user studies' shows each more than once
: test pilot thunderbird 'all your user studies' shows each more than once
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Thunderbird 13.0
Assigned To: Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-02 09:39 PST by Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )
Modified: 2012-02-15 16:58 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
screenshot showing doubled printing of results. (176.70 KB, image/png)
2012-02-02 09:39 PST, Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )
no flags Details
patch for bug. Neeeds review. (6.24 KB, patch)
2012-02-03 10:27 PST, Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )
squibblyflabbetydoo: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review

Description Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat ) 2012-02-02 09:39:48 PST
Created attachment 593887 [details]
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 ) 2012-02-02 09:51:12 PST
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) 2012-02-03 05:12:58 PST
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 ) 2012-02-03 10:27:24 PST
Created attachment 594237 [details] [diff] [review]
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) 2012-02-11 22:18:37 PST
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) 2012-02-13 06:30:45 PST
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 7 Jim Porter (:squib) 2012-02-15 16:58:57 PST
(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.

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