Closed Bug 825740 Opened 7 years ago Closed 7 years ago
Dev Tools does not remove the menuitems upon unregistering of a tool
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.
bad for addons. Promoted to P2.
Priority: P3 → P2
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
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]
https://hg.mozilla.org/integration/fx-team/rev/8893a63a88f8 This will need an uplift to Aurora.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
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+
You need to log in before you can comment on or make changes to this bug.