Closed
Bug 825740
Opened 11 years ago
Closed 11 years ago
[toolbox] gDevTools does not remove the menuitems upon unregistering of a tool
Categories
(DevTools :: Framework, defect, P2)
Tracking
(firefox20 fixed)
RESOLVED
FIXED
Firefox 21
Tracking | Status | |
---|---|---|
firefox20 | --- | fixed |
People
(Reporter: Optimizer, Assigned: Optimizer)
References
Details
Attachments
(1 file)
4.85 KB,
patch
|
paul
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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.
Updated•11 years ago
|
Priority: -- → P3
Assignee | ||
Comment 2•11 years ago
|
||
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)
Assignee | ||
Updated•11 years ago
|
Attachment #698156 -
Attachment is patch: true
Assignee | ||
Updated•11 years ago
|
Whiteboard: [has-patch]
Comment 3•11 years ago
|
||
Comment on attachment 698156 [details] [diff] [review] patch v1.0 r+, but why all these new if()?
Attachment #698156 -
Flags: review?(paul) → review+
Assignee | ||
Comment 4•11 years ago
|
||
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.
Assignee | ||
Updated•11 years ago
|
Whiteboard: [has-patch] → [land-in-fx-team]
Comment 5•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/8893a63a88f8 This will need an uplift to Aurora.
Updated•11 years ago
|
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8893a63a88f8
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 21
Assignee | ||
Comment 7•11 years ago
|
||
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 8•11 years ago
|
||
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+
Assignee | ||
Updated•11 years ago
|
Whiteboard: [land-in-aurora]
Comment 9•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/4767e3fcde95
status-firefox20:
--- → fixed
Whiteboard: [land-in-aurora]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•