Closed
Bug 740662
Opened 13 years ago
Closed 13 years ago
Replace InspectorStore dictionaries with Inspector instances
Categories
(DevTools :: Inspector, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 14
People
(Reporter: dcamp, Assigned: dcamp)
References
Details
Attachments
(1 file, 1 obsolete file)
23.68 KB,
patch
|
Details | Diff | Splinter Review |
Bug 707809 exposes a new Inspector object that's supposed to have the same lifetime as an InspectorStore. Inspector should replace InspectorStore.
Assignee | ||
Updated•13 years ago
|
Summary: Merge Inspector and InspectorStore objects → Replace InspectorStore dictionaries with Inspector instances
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #612391 -
Flags: review?(rcampbell)
Comment 2•13 years ago
|
||
Comment on attachment 612391 [details] [diff] [review]
v1
toggleHTMLPanel: function TP_toggle()
{
if (this.treePanel.isOpen()) {
this.treePanel.close();
Services.prefs.setBoolPref("devtools.inspector.htmlPanelOpen", false);
- this.store.setValue(this.winID, "htmlPanelOpen", false);
+ this.currentInspector._htmlPanelOpen = false;
} else {
this.treePanel.open();
Services.prefs.setBoolPref("devtools.inspector.htmlPanelOpen", true);
- this.store.setValue(this.winID, "htmlPanelOpen", true);
+ this.currentInspector._htmlPanelOpen = true;
}
},
this method still bugs me.
I think we could do something like:
toggleHTMLPanel() {
let wasClosed = !this.treePanel.isOpen();
if (wasClosed)
this.treePanel.open();
else
this.treePanel.close();
Services.prefs.setBoolPref("devtools.inspector.htmlPanelOpen", wasClosed);
this.store.setValue(this.winID, "htmlPanelOpen", wasClosed);
this.currentInspector._htmlPanelOpen = wasClosed;
}
I'm not sure this is worth the change or if this is more or less confusing than the duplicated methods though. Dealer's choice.
- this._currentInspector = new Inspector(this);
+ this.initializeStore();
I thought we were getting rid of InspectorStore...
I see what you did there. Repurposing it to store the inspector instances. Looks good.
does browser_inspector_tab_switch.js just work with this new implementation? If so, that's pretty sweet.
Should probably send it through try before landing.
Attachment #612391 -
Flags: review?(rcampbell) → review+
Assignee | ||
Comment 3•13 years ago
|
||
(In reply to Rob Campbell [:rc] (:robcee) from comment #2)
> this method still bugs me.
>
> I think we could do something like:
> I'm not sure this is worth the change or if this is more or less confusing
> than the duplicated methods though. Dealer's choice.
>
Yeah, that makes sense. I'll do it.
> - this._currentInspector = new Inspector(this);
> + this.initializeStore();
>
> I thought we were getting rid of InspectorStore...
>
> I see what you did there. Repurposing it to store the inspector instances.
> Looks good.
>
> does browser_inspector_tab_switch.js just work with this new implementation?
> If so, that's pretty sweet.
On my machine...
>
> Should probably send it through try before landing.
Indeed.
Assignee | ||
Comment 4•13 years ago
|
||
Unbitrot (jimb and I spent a little while trying to find a reverse of 'rot', but this is the best we could find)
Attachment #612391 -
Attachment is obsolete: true
Assignee | ||
Comment 5•13 years ago
|
||
Assignee | ||
Comment 6•13 years ago
|
||
Whiteboard: [fixed-in-fx-team]
Comment 7•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 14
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•