Bug 19207

Summary: Inspector should remember the size of sidebars set by the user
Product: WebKit Reporter: Anthony Ricaud <rik>
Component: Web Inspector (Deprecated)Assignee: Yury Semikhatsky <yurys>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, dev+webkit, joepeck, masterov, pfeldman, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 41687    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch pfeldman: review+

Anthony Ricaud
Reported 2008-05-23 02:54:04 PDT
It's a user preference, it should be remembered over instances.
Attachments
Patch (15.82 KB, patch)
2010-07-06 23:43 PDT, Yury Semikhatsky
no flags
Patch (16.57 KB, patch)
2010-07-07 01:53 PDT, Yury Semikhatsky
no flags
Patch (18.63 KB, patch)
2010-07-07 02:15 PDT, Yury Semikhatsky
pfeldman: review+
Anthony Ricaud
Comment 1 2008-07-16 11:16:02 PDT
I'd like to work on the "remember" bugs (bug 19208 and bug 19209). I need a way to store these preferences. I've thought about Database Storage and localStorage. Maybe there's also a way to do this in WebKit.
Timothy Hatcher
Comment 2 2008-07-16 12:04:17 PDT
I would say localStorage is the best bet. But it would be lumped into all file URL's localStorage. So Database with a unique name for the inspector might be better.
Matt Lilek
Comment 3 2008-08-02 15:23:15 PDT
(In reply to comment #2) > I would say localStorage is the best bet. But it would be lumped into all file > URL's localStorage. So Database with a unique name for the inspector might be > better. > Database is async, which is a problem when retrieving the information, so unless I'm missing a really obvious (or sneaky) way of making the database stuff synchronous, it's probably going to have to be localStorage.
Timothy Hatcher
Comment 4 2008-08-20 14:44:15 PDT
(In reply to comment #3) > (In reply to comment #2) > > I would say localStorage is the best bet. But it would be lumped into all file > > URL's localStorage. So Database with a unique name for the inspector might be > > better. > > > > Database is async, which is a problem when retrieving the information, so > unless I'm missing a really obvious (or sneaky) way of making the database > stuff synchronous, it's probably going to have to be localStorage. > Yes asynchronous makes it annoying. But you can still use it. When the Inspector opens, read all the preferences from the database into a Preferences object. Then when a preferences needs to change store the preference in the database and in the Preferences object. Any clients of the preferences will read only from the Preferences object, but the values do still persist. This is basically how localStorage works behind the scenes. Maybe we could consider a way to tell the Web Inspector's WebKit to use a special backing database for it's localStorage, so it does not conflict with other file URLs?
Alexei Masterov
Comment 5 2010-05-24 11:16:49 PDT
Web Inspector: [REGRESSION]: Styles Sidebar (Panel) would NOT re-open with the width set by the user in the previous session. To reproduce: ============== 1. Open Elements Panel. 2. Change the layout (width for example) of the sidebar 3. Close Inspector 4. Open Inspector 5. Observe sidebar return to the default layout. NOTE: previous version of WebKit had this working.
Alexei Masterov
Comment 6 2010-07-06 07:14:07 PDT
This bug is more general than 41687, so 41687 is set as a dependency
Yury Semikhatsky
Comment 7 2010-07-06 23:43:00 PDT
Pavel Feldman
Comment 8 2010-07-06 23:56:39 PDT
Comment on attachment 60691 [details] Patch WebCore/inspector/front-end/Panel.js:35 + this._name = name; Nit: it is easy for ancestors to hide it. I'd rename it to something more specific (_panelName) WebCore/inspector/front-end/Panel.js:42 + get toolbarItemClass() No need to expose it, just use the name in get toolbarItem below. WebCore/inspector/front-end/Panel.js:327 + get _sidebarWidthSettingName() I'd not use getters for private fields (use function instead). WebCore/inspector/front-end/Panel.js:392 + WebInspector.log("saving " + this._sidebarWidthSettingName + " = " + this.sidebarElement.offsetWidth); Remove logging. WebCore/inspector/front-end/Settings.js:61 + WebInspector.applicationSettings._installSetting("elementsSidebarWidth", "elements-sidebar-width", undefined); I think panel constructors should register theses instead - otherwise hack. r- for this. I'd make installSetting public and allow its invocation before the load takes place. I'd do the same thing as now + initialize store with default values. Then I'd populate existing settings from JSON.parse(settingsString) in _load.
Yury Semikhatsky
Comment 9 2010-07-07 01:53:21 PDT
Yury Semikhatsky
Comment 10 2010-07-07 02:15:22 PDT
Yury Semikhatsky
Comment 11 2010-07-07 02:15:56 PDT
Addressed all comments. Please take a look.
Yury Semikhatsky
Comment 12 2010-07-07 03:10:46 PDT
Committed r62647
Note You need to log in before you can comment on or make changes to this bug.