Closed Bug 598923 Opened 9 years ago Closed 9 years ago

add-on bar should be made visible if there are any add-ons installed

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 4.0b8
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: dietrich, Assigned: dietrich)

References

Details

(Whiteboard: [addon bar])

Attachments

(1 file, 3 obsolete files)

the patch originally had mutation event listeners that toggled the visibility of the bar as needed. however, those were removed due to perf/security concerns.

for now we should check at window load, and if there's a way to listen for addon installation, then too.
Blocks: 574688
This doesn't make sense to me. I thought the Addon bar was meant to be purely optional? Perhaps it'd be better to offer a door-hanger notification that will allow users to turn it on?
The interaction I'm talking about is just when a new add-on is installed that puts itself in the bar. We'd open the bar then. This gives the user visual confirmation of what happened to the add-on they just installed.

We could also do something fancy like flash the new one and then slide the bar out of view after a second. But that leads to frustrations for users that then have to go figure out how to turn it back on (hidden in a submenu!).
Requesting blocking - without this, users might install add-ons that use the add-on bar, and never ever see them. Ever.
Assignee: nobody → dietrich
blocking2.0: --- → ?
(In reply to comment #3)
> Requesting blocking - without this, users might install add-ons that use the
> add-on bar, and never ever see them. Ever.

That's just given me a thought. Would it be possible to have Addon ask users whether they want their button added to the navigation bar or the Addon bar on Installation? Apologies if I'm over-thinking things.
blocking2.0: ? → betaN+
Attached patch wip (obsolete) — Splinter Review
Attachment #482125 - Flags: feedback?(dao)
(In reply to comment #4)
> That's just given me a thought. Would it be possible to have Addon ask users
> whether they want their button added to the navigation bar or the Addon bar on
> Installation? Apologies if I'm over-thinking things.

I'd prefer zero interruption + post-install customization over interrupting the user. If it's totally confusing to people then let's revisit our approach at that point.
(In reply to comment #6)
> (In reply to comment #4)
> > That's just given me a thought. Would it be possible to have Addon ask users
> > whether they want their button added to the navigation bar or the Addon bar on
> > Installation? Apologies if I'm over-thinking things.
> 
> I'd prefer zero interruption + post-install customization over interrupting the
> user. If it's totally confusing to people then let's revisit our approach at
> that point.

I'm a "power user" so I'll think to check the customisation palette, however for users that aren't using the nightlies and haven't followed the development of the Addon Bar, the buttons will be easily lost.

Alternatives are Doorhangers, but they disappear if you even sneeze on the browser, so you can't notify users of the ability to add a button there. Where as prompting users as to where they'd like to place the button follows established Microsoft installation familiarity.
(In reply to comment #7)
> I'm a "power user" so I'll think to check the customisation palette, however
> for users that aren't using the nightlies and haven't followed the development
> of the Addon Bar, the buttons will be easily lost.

I don't understand. The whole point of this bug is about showing the bar after an add-on that puts itself on the bar is installed. That's what this patch does, exactly so what you described does not happen.

I'm not saying it's perfect, nor what we'll ultimately ship with, however it achieves two goals:

1. Shows users that the add-on bar exists

2. Shows users where that add-on they just installed went to

Once this lands, let's take a look at the user feedback.

Also, you bring up a great point: For add-ons that don't pre-install themselves anywhere, so are only on the palette, we might want to do something for discoverability - maybe a doorhanger as you suggest. Can you file a separate bug for that?

> Alternatives are Doorhangers, but they disappear if you even sneeze on the
> browser, so you can't notify users of the ability to add a button there.

I like this idea - to not only show the add-on bar, but to also put a doorhanger off the new add-on, that summarizes the customization options.

File a separate bug for the shortcomings of doorhangers please.

> Where as prompting users as to where they'd like to place the button follows
> established Microsoft installation familiarity.

That doesn't make it right. And I don't want to go down the road we've been speeding away from: prompting the user, interrupting their workflow, instead of achieving a design that just works in most scenarios.
Comment on attachment 482125 [details] [diff] [review]
wip

In order to persist the 'collapsed' state, you should call setToolbarVisibility instead of setting 'collapsed' on the toolbar.
Attachment #482125 - Flags: feedback?(dao) → feedback+
(In reply to comment #8)
> (In reply to comment #7)
> > I'm a "power user" so I'll think to check the customisation palette, however
> > for users that aren't using the nightlies and haven't followed the development
> > of the Addon Bar, the buttons will be easily lost.
> 
> I don't understand. The whole point of this bug is about showing the bar after
> an add-on that puts itself on the bar is installed. That's what this patch
> does, exactly so what you described does not happen.
> 
> I'm not saying it's perfect, nor what we'll ultimately ship with, however it
> achieves two goals:
> 
> 1. Shows users that the add-on bar exists
> 
> 2. Shows users where that add-on they just installed went to
> 
> Once this lands, let's take a look at the user feedback.
> 
> Also, you bring up a great point: For add-ons that don't pre-install themselves
> anywhere, so are only on the palette, we might want to do something for
> discoverability - maybe a doorhanger as you suggest. Can you file a separate
> bug for that?
Bug 603185

> > Alternatives are Doorhangers, but they disappear if you even sneeze on the
> > browser, so you can't notify users of the ability to add a button there.
> 
> I like this idea - to not only show the add-on bar, but to also put a
> doorhanger off the new add-on, that summarizes the customization options.
Bug 603187 

> File a separate bug for the shortcomings of doorhangers please.
Bug 603188 

Not sure if I was eloquent enough, but all filed.
Whiteboard: [needs new patch]
Attached patch v1 (obsolete) — Splinter Review
Attachment #482125 - Attachment is obsolete: true
Whiteboard: [needs new patch] → [needs test]
Attached patch v1.1 (obsolete) — Splinter Review
patch with test.
Attachment #487387 - Attachment is obsolete: true
Attachment #487543 - Flags: review?(dao)
Whiteboard: [needs test] → [needs review dao]
Comment on attachment 487543 [details] [diff] [review]
v1.1

>+  finish();

Shouldn't be needed.
Attachment #487543 - Flags: review?(dao) → review+
Attachment #487543 - Attachment is obsolete: true
Attachment #488151 - Flags: review+
Whiteboard: [needs review dao] → [has patch][can land]
From what I understand from just quickly reading the comments is that this patch will show the add-on bar after a add-on that puts itself in to the add-on bar is installed; so then my question would be how should a add-on put itself into the add-on bar?

If I overlay it, then the user can't move the toolbar button I inserted; if I use this hack https://developer.mozilla.org/en/Code_snippets/Toolbar#Adding_button_by_default for the add-on bar then will your patch understand that my add-on has put itself into the add-on bar?

Also, is there or will there be some better way than the mentioned hack to add a toolbar button to the add-on bar by default?
Page for best-practices in adding to the add-on bar is started here, not done yet though. https://wiki.mozilla.org/User:Dietrich/Scratchpad

This patch listens for add-on install/uninstall events, counts the child elements before and after, to determine whether to show/hide. This should handle any method of adding to the add-on bar.

It doesn't handle scenarios unrelated to add-on install/uninstall.
(In reply to comment #16)
> This patch listens for add-on install/uninstall events, counts the child
> elements before and after, to determine whether to show/hide. This should
> handle any method of adding to the add-on bar.
> 
> It doesn't handle scenarios unrelated to add-on install/uninstall.

You are referring to the install/uninstall events which trigger the install/uninstall methods of a extension with a bootstrap.js I assume? or can a regular extension hook into these events?
(In reply to comment #17)
> You are referring to the install/uninstall events which trigger the
> install/uninstall methods of a extension with a bootstrap.js I assume? or can a
> regular extension hook into these events?

Nvm, I forgot for a moment that a reg extension would req a restart, so these events would have to be fired then, and I'm pretty certain that's not being done.
Whiteboard: [has patch][can land] → [has patch][can land][addon bar]
http://hg.mozilla.org/mozilla-central/rev/6b25735eebd0
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][can land][addon bar] → [addon bar]
Verified with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101117 Firefox/4.0b8pre ID:20101117030837

A Litmus and Mozmill test will be part of the basic add-ons manager tests.
Status: RESOLVED → VERIFIED
Flags: in-litmus?(hskupin)
Target Milestone: --- → Firefox 4.0b8
Perhaps I'm missing something, but I can't get this to work.

I've tried installing an uninstalling extensions that add elements directly to both the addon-bar and status-bar via overlay. However, the addon-bar is never shown or hidden on restart.

Mozilla/5.0 (X11; Linux x86_64; rv:2.0b8pre) Gecko/20101120 Firefox/4.0b8pre

STR:

1. Start Minefield with a new profile
2. Install Status-4-Evar
3. Restart Minefield

Expected results:

The Addon-Bar is shown

Actual results:

The Addon-Bar is not shown
Same here, just doesn't work.  I checked and the patch exists in my build, but a quick scan suggests that it will only work for restartless addons?
I just tested this with an add-on that adds a Jetpack widget, and the add-on bar became visible after installation, and was hidden when I uninstalled.

I am seeing some odd behavior though: I noticed that I seeing a "lastAddonBarCount is undefined" error in onUninstalling. So I though dump() statements in each method of the addons manager listener. The install methods dump. The uninstall methods do not! But I know they're getting called, since the bar does go away on uninstall. Still investigating.
I haven't tested this with JetPacks. The problem is with regular extensions. My guess is that:
A) The listener is being registered after the install events for regular add-ons occur
B) The install events happen before the toolbar items are overlaid onto the toolbar

Rather than using an AddonManager listener, wouldn't it be simpler and more robust to persist the expected item count as an attribute on the toolbar, then handle this directly in BrowserStartup(), delayedStartup(), or on window load/showing?
Yeah, this patch clearly doesn't work for non-restartless extensions. They
insert their toolbar buttons on window load after a restart - long after
onInstalled is called and when lastAddonBarCount isn't even remembered any more.
Filed bug 616419 for the non-restartless case.
Flags: in-litmus?(hskupin) → in-litmus?(ioana.budnar)
Testcase #45481 has been added in Litmus for:
aurora basic functional tests (bfts) (276) -> fx aurora bft - add-ons (2173)
aurora full functional tests (ffts) (275) -> fx aurora fft - add-ons (2174).

Henrik, please let me know if you want me to add the testcase to any other test suites.
Flags: in-litmus?(ioana.budnar) → in-litmus+
(In reply to Ioana Budnar [QA] from comment #27)
> Testcase #45481 has been added in Litmus for:
> aurora basic functional tests (bfts) (276) -> fx aurora bft - add-ons (2173)
> aurora full functional tests (ffts) (275) -> fx aurora fft - add-ons (2174).

Thanks Ioana, but this test should go under Toolbar and not Add-ons. Can you please move it to that subgroup? Thanks.
Henrik, there is no Toolbar subgroup under BFTs. The only subgroup close to that is Toolbar Customization which only contains testcases about customizing toolbars. This is why I added the testcase under Add-ons. Please let me know if you want me to move the testcase under Toolbar Customization.
You need to log in before you can comment on or make changes to this bug.