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

RESOLVED FIXED in 1.1

Status

Add-on SDK
General
P1
normal
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: irakli)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [addon bar])

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
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.
(Reporter)

Updated

8 years ago
blocking2.0: --- → ?
Bug 623965 and bug 607600 both both implicate firebug is causing this. Does it also happen in safe mode?
(Reporter)

Comment 2

8 years ago
(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

8 years ago
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

8 years ago
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?

Updated

8 years ago
OS: Mac OS X → All
Summary: Add-on bar gets displayed on every restart after being manually closed → Add-on bar toolbar gets displayed on every restart after being manually closed
Version: unspecified → Trunk
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?
Created attachment 506068 [details]
Pointer to pull request
Attachment #506068 - Flags: review?(myk)
Duplicate of this bug: 623965
Duplicate of this bug: 607600
blocking2.0: ? → betaN+
Whiteboard: [addon bar] → [addon bar][hardblocker]
this seems to have a owner, Dietrich please correct if I'm wrong.
Assignee: nobody → dietrich
This is a Jetpack bug. Sorry for not reclassifying it when I attached the patch.
blocking2.0: betaN+ → ---
Component: Toolbars → General
Product: Firefox → Add-on SDK
QA Contact: toolbars → general
Whiteboard: [addon bar][hardblocker] → [addon bar]
Version: Trunk → unspecified
(Reporter)

Comment 12

8 years ago
(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...
I don't have any Jetpack installed either and I am affected by this bug (with Firebug icon)
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.
(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.
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
Pascal: what's in your add-on bar?
the firebux icon
I confirmed in a new profile that the latest Firebug does indeed cause this behavior.
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!
Attachment #506068 - Flags: review?(myk) → review?(adw)

Comment 21

8 years ago
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?
Attachment #506068 - Flags: review?(adw) → review-

Comment 22

8 years ago
(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.
(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

8 years ago
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

8 years ago
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.
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

8 years ago
(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.
Duplicate of this bug: 630637
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.
I updated the pull request with another change: the widget code was closing the add-on bar when there were no more widgets left.
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

8 years ago
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.
(Reporter)

Comment 33

8 years ago
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.
(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.
Filed bug 631470 for the cfx run problem.
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.
Since this isn't in a component that can block, should we file an analogous Firefox bug pointing over here?
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

8 years ago
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.
Summary: Add-on bar toolbar gets displayed on every restart after being manually closed → Add-on bar toolbar gets displayed on every restart after being manually closed (with add-ons created by the Add-on SDK)
The fix is coming in Alex's work in bug 617546.
Assignee: dietrich → nobody
Depends on: 617546
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 ...
Depends on: 631470
Priority: -- → P2
Hardware: x86 → All
Target Milestone: --- → 1.0
(automatic reprioritization of 1.0 bugs)
Priority: P2 → P1
Target Milestone: 1.0 → 1.1
Assignee: nobody → rFobic
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.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.