Closed Bug 825740 Opened 7 years ago Closed 7 years ago

[toolbox] gDevTools does not remove the menuitems upon unregistering of a tool

Categories

(DevTools :: Framework, defect, P2)

x86_64
Windows 7
defect

Tracking

(firefox20 fixed)

RESOLVED FIXED
Firefox 21
Tracking Status
firefox20 --- fixed

People

(Reporter: Optimizer, Assigned: Optimizer)

References

Details

Attachments

(1 file)

Register a tool using gDevTools.regisrTools(toolDefinition) with an id "testing" (or any other thing).

Remove the tool using gDevTools.unregisterTool("testing")

The tool is gone from the toolbox, but the menu items still remain.
Priority: -- → P3
bad for addons. Promoted to P2.
Priority: P3 → P2
Attached patch patch v1.0Splinter Review
Updated the test to check for menu item too. Did not add key and appmenu as they are subject to tool definition and the operating system respectively.
Attachment #698156 - Flags: review?(paul)
Attachment #698156 - Attachment is patch: true
Whiteboard: [has-patch]
Comment on attachment 698156 [details] [diff] [review]
patch v1.0

r+, but why all these new if()?
Attachment #698156 - Flags: review?(paul) → review+
Let me explain all of them:

the if(radio) change is just a redundancy removal.

the if(appmenuitem) is for OSX

the rest of them are there because of the fact that add-ons might misbehave sometimes and due to which there can be uncertainty. This function is mostly used by add-ons only, so Checking once will prevent from erroring out prematurely and thus leaking memory, as the remaining entries might have needed a removal.

Also, as the  gDevTools._addToolToMenu method has a way of not appending, but just returning the keys and menu items, we are never sure what actually got attached.

I might be wrong with my hypothesis here.
Whiteboard: [has-patch] → [land-in-fx-team]
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/8893a63a88f8
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 21
Comment on attachment 698156 [details] [diff] [review]
patch v1.0

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 788977
User impact if declined: Various Menu items for add-on that extend the toolbox would not get removed
Testing completed (on m-c, etc.): tested on m-c
Risk to taking this patch (and alternatives if risky): no risk
String or UUID changes made by this patch: no string change
Attachment #698156 - Flags: approval-mozilla-aurora?
Comment on attachment 698156 [details] [diff] [review]
patch v1.0

Low risk fix for a new dev tools feature. Approving for Aurora 20.
Attachment #698156 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [land-in-aurora]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.