Closed
Bug 890262
Opened 11 years ago
Closed 10 years ago
CustomizableUI.destroyWidget raises an exception after `CustomizableUI.addWidgetToArea`
Categories
(Firefox :: Toolbars and Customization, defect)
Tracking
()
RESOLVED
FIXED
Firefox 28
People
(Reporter: zer0, Assigned: Gijs)
References
(Blocks 1 open bug)
Details
(Keywords: addon-compat, Whiteboard: [Australis:M8][Australis:P2])
Attachments
(2 files, 2 obsolete files)
1.37 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
3.42 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
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 CustomizableUIInternal.destroyWidget(aWidgetId); 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.
Reporter | ||
Updated•11 years ago
|
Blocks: australis-cust
Whiteboard: [Australis:M?]
Assignee | ||
Comment 1•11 years ago
|
||
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. :-)
Assignee | ||
Updated•11 years ago
|
Keywords: addon-compat
Whiteboard: [Australis:M?] → [Australis:P2]
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Assignee | ||
Comment 2•11 years ago
|
||
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)
Assignee | ||
Comment 3•11 years ago
|
||
For reference, this was my test (the opening window thing was a check to see if that helped; it didn't).
Assignee | ||
Comment 4•11 years ago
|
||
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)
Updated•11 years ago
|
Attachment #773322 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 5•11 years ago
|
||
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 6•11 years ago
|
||
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+
Assignee | ||
Comment 7•11 years ago
|
||
Folded and pushed: https://hg.mozilla.org/projects/ux/rev/ec67f6224dd9
Status: NEW → ASSIGNED
Whiteboard: [Australis:P2] → [Australis:P2][fixed-in-ux]
Assignee | ||
Updated•11 years ago
|
Whiteboard: [Australis:P2][fixed-in-ux] → [Australis:M8][Australis:P2][fixed-in-ux]
Assignee | ||
Comment 8•11 years ago
|
||
This was backed out yesterday: https://hg.mozilla.org/projects/ux/rev/a8a320338f33
Whiteboard: [Australis:M8][Australis:P2][fixed-in-ux] → [Australis:M8][Australis:P2]
Assignee | ||
Comment 9•11 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #8) > This was backed out yesterday: > https://hg.mozilla.org/projects/ux/rev/a8a320338f33 Sorry, this was inaccurate; the work was left in, but the test backed out. Relanded as: https://hg.mozilla.org/projects/ux/rev/56af4ca8e542
Whiteboard: [Australis:M8][Australis:P2] → [Australis:M8][Australis:P2][fixed-in-ux]
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ec67f6224dd9
Status: ASSIGNED → RESOLVED
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.
Description
•