993322 - CustomizableUI: Toolbar icon disappear if a toolbar is removed

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Yvon TANGUY]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140317233623

Steps to reproduce:

Steps to reproduce:
1) Install an extension which add toolbar, like:
- "The Addon Bar (restored)" (https://addons.mozilla.org/addon/the-addon-bar/)
- "Classic Theme Restorer (Customize Australis)" (https://addons.mozilla.org/addon/classicthemerestorer/)

2) Install an extension which add a toolbar icon, like the "Australis View Test 0.1" example found in: https://blog.mozilla.org/addons/2014/03/06/australis-for-add-on-developers-2/

3) Put the extension icon in a new toolbar (like the addon bar).
4) Optionnaly restart Firefox to be sure everything is saved.
5) Disable the "toolbar" extension (for "Classic Theme Restorer" you will need to restart Firefox)





Actual results:

The addon button disapear completely. Even after restart. It is not available in the toolbox neither.


Expected results:

Add-ons buttons should be moved to the toolbox or to their default location (usually the navbar).
Workaround: Enter the customize mode, then click on the restore default configuration button.

Comment 1

[Yvon TANGUY]

This bug may be related to, or at least have some things in common with bug #989609

Comment 2

[:Gijs (he/him)]

Umm, I actually can't reproduce this issue. Here's what I did:

(In reply to Yvon TANGUY from comment #0)
> 1) Install an extension which add toolbar, like:
> - "The Addon Bar (restored)"
> (https://addons.mozilla.org/addon/the-addon-bar/)
> 2) Install "Australis View Test 0.1" example found in:
> https://blog.mozilla.org/addons/2014/03/06/australis-for-add-on-developers-2/
> 
> 3) Put the extension icon in the new "add-on bar" that the first addon added
> 4) Disable the "toolbar" extension
> 5) Open customization mode

The button then shows up in customization mode's toolbox.

Yvon, can you clarify what I'm missing?

Comment 3

[Yvon TANGUY]

You are right, if you don't restart Firefox with "The Addon Bar (restored)" extension, the bootstrap's add-on icon is restored in the toolbox.
But not if you restart Firefox. I just retried. 
I said "Optionally restart Firefox to be sure everything is saved.", remove the word "Optionally" from the sentence... sorry.

For the "Classic Theme Restorer (Customize Australis)" add-on, since it is not a bootstrap add-on, you have to restart anyway.

To restart, for a test with "The Addon Bar (restored)", I just quit FFx, then relaunch it.

Comment 4

[:Gijs (he/him)]

Attachment: fix widgets not showing up in toolbox,

:-(

Comment 5

[:Gijs (he/him)]

FWIW, an alternative fix here is to make currentArea be null if the area isn't registered (anymore) but I don't know that that would be better. Blair, can you think about this carefully? It's 1am here and I'm crashing, but at least this works, has a working test (that breaks on current fx-team tip, including bug 989609's fix) and it makes the button show up with the manual testcase in comment #0 / comment #3.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8403668 [details] [diff] [review]
fix widgets not showing up in toolbox,

Review of attachment 8403668 [details] [diff] [review]:
-----------------------------------------------------------------

Consider this scenario:

1) Add-on toolbar is registered
2) Buttons are moved into that toolbar by the user
3) Add-on toolbar is unregistered and destroyed
4) Browser is restarted
5) Add-on toolbar is re-registered

Before step 5, I *believe* the currentArea and positions for the widgets moved into the toolbar in step 2 are not set. My patch in bug 989609 made it so that step 5 causes the currentArea and position to be set, and we're all hunky dory.

So my gut instinct here says to try to be internally consistent - when a widget is registered, if its current placement area does not exist, its currentArea and position should not be set.

So that'd be a change inside createWidget. Just my 2c, anyway.

Comment 7

[:Gijs (he/him)]

Attachment: fix widgets not showing up in toolbox,

Per discussion on IRC, I believe this works well. Certainly passes the same test (and all the other ones) on my mac.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8404770 [details] [diff] [review]
fix widgets not showing up in toolbox,

Review of attachment 8404770 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine by me. Thanks Gijs!

::: browser/components/customizableui/test/browser_993322_widget_notoolbar.js
@@ +5,5 @@
> +"use strict";
> +
> +const BUTTONID = "test-API-created-widget-toolbar-gone";
> +const TOOLBARID = "test-API-created-extra-toolbar";
> +add_task(function*() {

Nit - please add a newline above this add_task, and remove the one below.

Comment 9

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/ddc43b493d75

Comment 10

[:Gijs (he/him)]

Comment on attachment 8404770 [details] [diff] [review]
fix widgets not showing up in toolbox,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis
User impact if declined: widgets can go missing if they were on add-on-created toolbars and those toolbars were removed
Testing completed (on m-c, etc.): (soon) on m-c, has automated test
Risk to taking this patch (and alternatives if risky): medium-low. It's late in the cycle, but on the other hand, having items go missing completely is quite serious, and fortunately this has an automated test.
String or IDL/UUID changes made by this patch: none

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ddc43b493d75

Comment 12

[:Gijs (he/him)]

https://hg.mozilla.org/releases/mozilla-beta/rev/c1bbbe2e1309
https://hg.mozilla.org/releases/mozilla-aurora/rev/4eac2e379747

Comment 13

[Cornel Ionce [:noni]]

Verified fixed on Windows 7 64bit, Windows 8.1 64bit (Microsoft Surface Pro 2), Ubuntu 12.04 32bit and Mac OS X 10.9 using:
#latest Nightly, build ID: 20140414030203
#latest Aurora, build ID: 20140415004003
#Fx 29 beta 8, build ID: 20140414143035