Closed Bug 861908 Opened 11 years ago Closed 11 years ago

Destroying Inspector without destroying Toolbox fails.

Categories

(DevTools :: Inspector, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 23

People

(Reporter: Optimizer, Assigned: Optimizer)

References

Details

Attachments

(1 file)

The STRs were not directly possible before, but once Options panel lands, the default tools' destroy method could be called before Toolbox's destroy.

In case of inspector, follow these STR using a latest Nightly or Fx-team:

1. Inspect any node
2. Open scratchpad in browser mode and run this gist : https://gist.github.com/anonymous/5389102
3. Fund the below error in error console : 

Timestamp: 4/15/2013 9:19:59 PM
Error: TypeError: this._tabbox.tabpanels is undefined: ToolSidebar_destroy@resource://app/modules/devtools/Sidebar.jsm:196
InspectorPanel__destroy@resource://app/modules/devtools/InspectorPanel.jsm:337
TBOX_toolUnregistered@resource://app/modules/devtools/Toolbox.jsm:674
EventEmitter_emit@resource:///modules/devtools/EventEmitter.jsm:100
@Scratchpad/1:11

Source File: resource:///modules/devtools/EventEmitter.jsm
Line: 105
Destroy is bizarre because we have both an event and a promise for the same thing. I wonder if we can reduce this to just a promise.
That should help, but I am still investigating on why does the |this._tabbox.tabpanels| is perfectly fine while closing toolbox but is undefined while doing it manually. I can see the sidebar perfectly fine in both cases.
So, this is because the _toolUnregistered method of Toolbox.jsm executes first and destroys the panel (DOM). Thus when the code flow reaches the destroy method of sidebar.jsm, the |this._tabbox| which is reference to a DOM element inside the panel is undefined.

I think this can simply be taken care by Sidebar.jsm's destroy method to null check |this._tabbox|
Assignee: nobody → scrapmachines
Status: NEW → ASSIGNED
Attached patch patch v0.1Splinter Review
Turns out that the toolbox destroy method was first removing the panel and then calling tool's destroy method. I don't know about other tools, but atleast Inspector relied on the panel to be present at its destroy method call.

All tests pass locally. Pushed to try at : https://tbpl.mozilla.org/?tree=Try&rev=3c767597da5e
Attachment #738387 - Flags: review?(jwalker)
Attachment #738387 - Flags: review?(jwalker) → review+
Thanks! I'll add this to my list of things to land unless anyone else gets there first.
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/ef07def8f957
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 23
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: