| Summary: | Web Inspector: Scopes sidebar should use new ObjectTreeView and not ObjectPropertiesSection | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||||
| Component: | Web Inspector | Assignee: | Joseph Pecoraro <joepeck> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Joseph Pecoraro
2015-03-17 18:39:27 PDT
Created attachment 248895 [details]
[PATCH] Proposed Fix
Comment on attachment 248895 [details]
[PATCH] Proposed Fix
I think I have to do a little more testing (WeakMap / Set) cq?.
Created attachment 248896 [details]
[IMAGE] Before
Created attachment 248897 [details]
[IMAGE] After
Looks great! Comment on attachment 248895 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=248895&action=review > Source/WebInspectorUI/UserInterface/Views/ObjectTreeView.js:212 > + this._extraProperties = []; I wonder if you could define `this._extraProperties = [];` within the constructor. Defining instance variables at one place increases readability and would reduce the spec for "appendExtraPropertyDescriptor" to a minimum imho. > Source/WebInspectorUI/UserInterface/Views/ObjectTreeView.js:266 > + if (this._extraProperties) Just in case you follow my comment in L212: `if (this._extraProperties.length > 0)` Comment on attachment 248895 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=248895&action=review >> Source/WebInspectorUI/UserInterface/Views/ObjectTreeView.js:212 >> + this._extraProperties = []; > > I wonder if you could define `this._extraProperties = [];` within the constructor. Defining instance variables at one place increases readability and would reduce the spec for "appendExtraPropertyDescriptor" to a minimum imho. Having that empty array around for all object trees takes up memory. We tend to limit our object footprints and don't define rare used properties in the constructor. > ninja: error: '../../Source/WebInspectorUI/UserInterface/Views/ScopeVariableTreeElement.js', needed by 'DerivedSources/webkit2gtk/InspectorGResourceBundle.xml', missing and no known rule to make it
GTK is going to need a forced clean build after this lands. Arg.
|