614699 - installing 'cfx init' addon causes all windows to close

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Brian Warner [:warner :bwarner]]

While testing the patch for bug 588119, I ran into a problem:

 addon-sdk from current trunk (https://github.com/mozilla/addon-sdk/commit/b2412945484b6878fc53e2b32c3f301a51e9bf59)
 FF4.0b7
 OS-X 10.6

I built the 'cfx init' sample addon:

* source bin/activate
* mkdir t; cd t
* cfx init
* cfx xpi  (this tells me to re-run, as usual)
* cfx xpi
* cp t.xpi /tmp/

Then I launch FF4.0b7, open up a couple of windows and tabs, then use
Command-O to open /tmp/t.xpi . When I hit the pulsing blue "Install Now"
button on the confirmation dialog, the pulsing stops for about 3 seconds, the
dialog scrolls back up out of site, then suddenly all the windows close and
the Finder switches to a different application (exactly as if FF had suddenly
quit). However the FF icon is still present in the Dock. Clicking on it
causes a new start-page window to open up. When I get back to the Add-ons
Manager page, my t.xpi is installed.

This doesn't happen with SDK-0.9 and FF4.0b6: on those, browser stays up.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Urk.  That should not be the default experience for "my first addon."

Comment 2

[Brian Warner [:warner :bwarner]]

As an extra datapoint, if I comment out the widget.Widget() call in main.js (leaving onle four require()s and a console.log), then installing the addon does not cause all windows to close. However, removing  it after installation *does* cause all windows to close. Commenting out the require() statements (leaving just the console.log) allows deinstallation to proceed without closing all windows.

For reference, it requires widget, tabs, panel, and self.

I'm suspecting something in the cleanup-before-unload path, maybe closing things that it doesn't really own.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Attachment: pull request #52v1: fixes problem

There are two problems. First, the addon is using the old Widget API. Second, the Window API (entrained by the Tabs API) closes all windows when it is unloaded, which it shouldn't be doing, since the API will be unloaded when addons are disabled, and disabling an addon shouldn't cause all windows to close.

In addition to review from Dietrich, requesting feedback from Irakli in case there are additional considerations here.  Note that jetpack-core/lib/windows/tabs.js _destroyWindowTabTracker closes all tabs, which also doesn't seem right, although it doesn't cause this problem.

Comment 4

[Dietrich Ayala (:dietrich)]

Comment on attachment 493618 [details]
pull request #52v1: fixes problem

r=me. if you haven't, should run the testsuite to make sure the unload change doesn't break any assumptions made in tests.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #4)
> r=me. if you haven't, should run the testsuite to make sure the unload change
> doesn't break any assumptions made in tests.

I have, and it doesn't (although doing the same thing for the tabs API does, which we should look at eventually).

https://github.com/mozilla/addon-sdk/commit/220e542b7c5ad9c4f449ca6ef1dcd4292073192a