Closed Bug 890262 Opened 11 years ago Closed 10 years ago

CustomizableUI.destroyWidget raises an exception after `CustomizableUI.addWidgetToArea`


(Firefox :: Toolbars and Customization, defect)

Not set



Firefox 28


(Reporter: zer0, Assigned: Gijs)


(Blocks 1 open bug)


(Keywords: addon-compat, Whiteboard: [Australis:M8][Australis:P2])


(2 files, 2 obsolete files)

So, I'm not sure if it's a bug or I'm doing something wrong here.

In the test suite for the new Button in jetpack, that are using CustomizableUI, I'm testing that the appropriate icon is displayed depends if we are in panel or in the navbar. Because by default the button is added in the navbar, from my code I call:

  CustomizableUI.addWidgetToArea(widgetId, CustomizableUI.AREA_PANEL);

Then I test all the stuff I need, and I unload the jetpack loader – that calls the `CustomizableUI.destroyWidget`, and here the exception is reaised:

TypeError: buildArea is undefined
resource://app/modules/CustomizableUI.jsm 1492
[cut all the jetpack's module trace]
  File "resource://app/modules/CustomizableUI.jsm", line 1808, in 
  File "resource://app/modules/CustomizableUI.jsm", line 1492, in 
    for (let buildNode of buildArea) {

As workaround, currently I'm move back the the widget to its original area (NAVBAR) before destroying it:

  CustomizableUI.addWidgetToArea(widgetId, CustomizableUI.AREA_NAVBAR);

Notice that this exception is not raised if I manually move the button from the navbar to the panel.
Whiteboard: [Australis:M?]
I'm fairly sure this is because we initialize the panel lazily. Can you easily test what happens if you simulate a click event on the menu panel button (to open it, and close it again after, I guess) before calling destroy ? I suspect that would also fix it.

If I'm correct, that still means it's a bug on our side, as we should cope with not having any build areas. :-)
Keywords: addon-compat
Whiteboard: [Australis:M?] → [Australis:P2]
Assignee: nobody → gijskruitbosch+bugs
Attached patch Patch (obsolete) — Splinter Review
I wrote a test for this but it only tests the failure properly if nothing opens the panel (adding it to gBuildAreas). Given that our other tests do that, this isn't really easily testable, and given the size of the patch, I think it's unlikely we'll really regress this. I did test, using that test in isolation, that this is actually fixed.
Attachment #773316 - Flags: review?(mconley)
Attached patch Test (obsolete) — Splinter Review
For reference, this was my test (the opening window thing was a check to see if that helped; it didn't).
Attached patch PatchSplinter Review
Adding the right patch helps; sorry for the spam!
Attachment #773316 - Attachment is obsolete: true
Attachment #773316 - Flags: review?(mconley)
Attachment #773322 - Flags: review?(mconley)
Attachment #773322 - Flags: review?(mconley) → review+
Attached patch Tests v2Splinter Review
This test does work correctly even if another test has run previously, by not depending on the panel. Thanks for the excellent suggestion! :-)
Attachment #773318 - Attachment is obsolete: true
Attachment #773425 - Flags: review?(mconley)
Comment on attachment 773425 [details] [diff] [review]
Tests v2

Review of attachment 773425 [details] [diff] [review]:

LGTM! Woo, tests!
Attachment #773425 - Flags: review?(mconley) → review+
Folded and pushed:
Whiteboard: [Australis:P2] → [Australis:P2][fixed-in-ux]
Whiteboard: [Australis:P2][fixed-in-ux] → [Australis:M8][Australis:P2][fixed-in-ux]
This was backed out yesterday:
Whiteboard: [Australis:M8][Australis:P2][fixed-in-ux] → [Australis:M8][Australis:P2]
(In reply to :Gijs Kruitbosch from comment #8)
> This was backed out yesterday:

Sorry, this was inaccurate; the work was left in, but the test backed out. Relanded as:
Whiteboard: [Australis:M8][Australis:P2] → [Australis:M8][Australis:P2][fixed-in-ux]
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M8][Australis:P2][fixed-in-ux] → [Australis:M8][Australis:P2]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.