627484 - Add-on bar toolbar gets displayed on every restart after being manually closed (with add-ons created by the Add-on SDK)

Status: RESOLVED FIXED

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

Description

[(no longer active)]

STR:

1. Close the add-on bar from all of the open windows.
2. Restart Firefox.

The add-on bar gets added to all windows again.

Comment 1

[Luke Iliffe (Harlequin99)]

Bug 623965 and bug 607600 both both implicate firebug is causing this. Does it also happen in safe mode?

Comment 2

[(no longer active)]

(In reply to comment #1)
> Bug 623965 and bug 607600 both both implicate firebug is causing this. Does it
> also happen in safe mode?

This happens for me with Firebug.  In safe mode, I'd have no extensions, so the add-on bar wouldn't show up.

Comment 3

[Asa Dotzler [:asa]]

It happens for me without Firebug installed. I have only Add-on Compatibility Reporter, Adblock Plus, Add-on Builder Helper, and Hard blockers counter installed.

Comment 4

[Asa Dotzler [:asa]]

I've determined that of all my add-ons, it's only the Hard blockers counter add-on that triggers this behavior. Now that we have two cases, Firebug and Hard blockers counter, triggering this behavior, maybe we should escalate? 

Galdalf, do you have any ideas on what's different in your add-on (and presumably Firebug as well) that might trigger this?

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

I use AddonSDK's Widget that I believe to do not do an awesome job with toolbars persistency overall.

For example:
 * it reopens addon bar on each launch (which probably is just how it deals with initialization code)
 * it disappears from the toolbar you moved it to and reappears in the addon bar after restart

That's probably just the way Widget() initializes itself and maybe Firebug is reusing it?

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

I reported bug 627886

Comment 7

[Dietrich Ayala (:dietrich)]

Attachment: Pointer to pull request

Comment 10

[Marco Bonardo [:mak]]

this seems to have a owner, Dietrich please correct if I'm wrong.

Comment 11

[Dietrich Ayala (:dietrich)]

This is a Jetpack bug. Sorry for not reclassifying it when I attached the patch.

Comment 12

[(no longer active)]

(In reply to comment #11)
> This is a Jetpack bug. Sorry for not reclassifying it when I attached the
> patch.

Hmm, I don't have any Jetpack based add-ons which put icons in the add-on bar...

Comment 13

[Pascal Chevrel:pascalc]

I don't have any Jetpack installed either and I am affected by this bug (with Firebug icon)

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

There's one thing in this bug that is addon-sdk specific - when a widget is being unloaded, an addon-bar is being hidden.
That seems to be part of unloading of the Widget.

I believe that Firebug may be reusing some of the code from AddonSDK to load itself into an addon-bar - possibly even a Widget code itself.

Comment 15

[Dietrich Ayala (:dietrich)]

(In reply to comment #12)
> (In reply to comment #11)
> > This is a Jetpack bug. Sorry for not reclassifying it when I attached the
> > patch.
> 
> Hmm, I don't have any Jetpack based add-ons which put icons in the add-on
> bar...

Can you provide your add-on list? Any add-on built from the Add-on SDK which uses the Widget module will have this bug.

Comment 16

[Pascal Chevrel:pascalc]

Here is the list of addons I have:

Name                        Version Enabled

Mass Password Reset          1.04     false
FirefoxNotify                1.5.4    false
Prism for Firefox            1.0b3    false
Ubuntu Firefox Modifications 0.9rc2   false
F1 by Mozilla Labs           0.7.3    true
DOM Inspector                2.0.8    true
ChromeBug                    1.7.0a8  true
Firebug                      1.7X.0a9 true

Comment 17

[Dietrich Ayala (:dietrich)]

Pascal: what's in your add-on bar?

Comment 18

[Pascal Chevrel:pascalc]

the firebux icon

Comment 19

[Dietrich Ayala (:dietrich)]

I confirmed in a new profile that the latest Firebug does indeed cause this behavior.

Comment 20

[Dietrich Ayala (:dietrich)]

The attached patch fixes the problem for add-ons built with the SDK. Anything else should be reported to the add-on author.

If you can reproduce the problem with no add-ons installed whatsoever, then please file a Firefox bug for the problem, and cc me!

Comment 21

[Drew Willcoxon :adw]

Comment on attachment 506068 [details]
Pointer to pull request

This does fix the bug, but it prevents the add-on bar from appearing after you've installed a new widget.  That's nice behavior we want to keep, right?

I think the bar should remain hidden from the time the user hides it up until the time he installs a new widget.  When he does install a new widget, it should appear even if he had previously hidden it, and it should keep showing until he hides it again.

Wouldn't persisting the bar's hidden/collapsed state (via the XUL persist attribute) work?

Comment 22

[Drew Willcoxon :adw]

(In reply to comment #21)
> Wouldn't persisting the bar's hidden/collapsed state (via the XUL persist
> attribute) work?

... with the help of some JS of course.

Comment 23

[Dietrich Ayala (:dietrich)]

(In reply to comment #21)

> This does fix the bug, but it prevents the add-on bar from appearing after
> you've installed a new widget.  That's nice behavior we want to keep, right?

This is done in the browser now. When restartless add-ons are installed, and add to the add-on bar, and the bar was previously hidden, it will be shown. And hidden on uninstall if there are no more items left on the bar.

Comment 24

[Drew Willcoxon :adw]

Oh, sweet.  Has that landed yet?  I tested the patch with the newest nightly and the bar remained hidden after install.  Also, does it persist between restarts?  Like, if I install a widget and the bar is shown immediately, that's great, but if I don't hide it, it should still be shown after I restart.  Similar question about uninstall.

Comment 25

[Drew Willcoxon :adw]

OK, I was testing with cfx run, and in that case the add-on bar is not shown.  When I package my widget as an XPI and install, the bar is shown.  And when I restarted with the XPI, the bar was still shown, and when I hid the bar and restarted, the bar remained hidden.

But, when I uninstalled my XPI, the bar was hidden, but when I restarted, the (empty) bar was shown.  I guess that's a different bug.

Can you fix the cfx run behavior?  If I'm testing my widget add-on with it, it would be annoying to have to manually show the bar every time I cfx run.

Comment 26

[Dietrich Ayala (:dietrich)]

Hrm, yeah the debug workflow with cfx sucks after this patch.

I think the problem is that cfx doesn't install the add-on as a restartless one, so those events don't get sent.

I can't dig into that right now though.

Comment 27

[Drew Willcoxon :adw]

(In reply to comment #25)
> But, when I uninstalled my XPI, the bar was hidden, but when I restarted, the
> (empty) bar was shown.  I guess that's a different bug.

Filed bug 630634.  Dietrich, it sounds like the problem there is in Firefox, while the problem here is in the SDK, but I'll let you dupe that bug to this one if that's the right way to go.

Comment 29

[Dietrich Ayala (:dietrich)]

Thanks Drew. I can reproduce bug 630634 with those steps.

This doesn't sound like the scenario originally reported in this bug though, is a separate issue.

Comment 30

[Dietrich Ayala (:dietrich)]

I updated the pull request with another change: the widget code was closing the add-on bar when there were no more widgets left.

Comment 31

[Dietrich Ayala (:dietrich)]

I'll need to poke into cfx code to figure out why it's not sending the proper install notifications. Cc'd Atul... does cfx not install the add-on as a restartless one?

I'm less concerned with fixing the cfx run scenario than with fixing this bug though. I recommend we take these fixes, and address the problem with the cfx tool in a different bug.

Comment 32

[Atul Varma [:atul]]

Aye, "cfx run" installs the new addon as a restartless one, *but* it also starts a brand-new profile of Firefox with the add-on pre-installed, so certain notifications may or may not get sent, depending on how the Add-ons Manager works.

You can go to the add-on manager during "cfx run" and the add-on actually shows up as "Test App"--you can enable/disable it without restarting, if that helps.

Comment 33

[(no longer active)]

Here's a general comment: do we want to give extensions (jetpack-based or not) to be able to override user's choice in dismissing the add-on bar?  I thought that putting the user in charge was one of the goals of having the add-on bar in the first place.

Comment 34

[Dietrich Ayala (:dietrich)]

(In reply to comment #33)
> Here's a general comment: do we want to give extensions (jetpack-based or not)
> to be able to override user's choice in dismissing the add-on bar?  I thought
> that putting the user in charge was one of the goals of having the add-on bar
> in the first place.

We absolutely do. Add-ons are inherently strong in the choice department: A user has to *choose* to install it. And if they don't like it, they'll choose to uninstall it.

Comment 35

[Dietrich Ayala (:dietrich)]

Filed bug 631470 for the cfx run problem.

Comment 36

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug seems like a good candidate for nomination to be a Firefox 4 soft blocker.  With the recent changes to add back the status bar, we went from having a single bar to now consistently having two bars (three bars, if you used the find bar), all in an attempt to get the number down to zero.

Comment 37

[Alex Faaborg [:faaborg] (Firefox UX)]

Since this isn't in a component that can block, should we file an analogous Firefox bug pointing over here?

Comment 38

[Dietrich Ayala (:dietrich)]

Alex, bug 630634 is sorta related. But this bug is purely in add-ons generated by the add-on sdk that use the widget module. We don't need any kind of Firefox blocking bug for it.

We could definitely use a better way to track and prioritize add-on SDK issues though (cc'ing Myk).

Comment 39

[Atul Varma [:atul]]

I'm renaming the title of this bug to better disambiguate it from being a Firefox bug... If that was the wrong move, feel free to revert the change.

Comment 40

[Dietrich Ayala (:dietrich)]

The fix is coming in Alex's work in bug 617546.

Comment 41

[Alexandre Poirot [:ochameau]]

This bug has not been fixed by pull request 118/bug 617546,
because I had to let one explicit show request in SDK code:
https://github.com/mozilla/addon-sdk/commit/e2b1538c0ad4d408affaa2ca37f072af7b07a4c0#L4R451
In order to display the addon bar when we install a jetpack addon.

I could remove this show request and it will work fine on final addons,
but it won't work with cfx test nor cfx run.
Because following Firefox code won't work:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#8586
This code is not working because the AddonInstallListener is registered too late, after the cfx run addon is installed.
cfx run simply put a new addon in /profile/extensions directory, then the addon is going to be installed very early during firefox startup.

I see at least two solutions:
- Track previous toolbaritem count from previous firefox session
- try to hack cfx to install addon after firefox is completely loaded

I think hacking cfx could be really smart move as it will be more realistic behavior! But I don't have any concrete idea on how to achieve this ...

Comment 42

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

(automatic reprioritization of 1.0 bugs)

Comment 43

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

I don't think it makes sense to keep this bug opened as issue described have been already fixed. Even though behavior when running with cfx is different it has nothing to do with this bug itself. When we run on cfx addons we are not even creating .xpi files and when we run tests we fallback to runtime module search logic, but all of those are totally different issues and have separate bugs.

I'm closing this bug as fixed since issue described in title is fixed, feel free to reopen if you disagree.