Bug 19207 – Inspector should remember the size of sidebars set by the user

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Anthony Ricaud]

It's a user preference, it should be remembered over instances.

[Anthony Ricaud]

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]

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]

(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]

(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]

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]

This bug is more general than 41687, so 41687 is set as a dependency

[Yury Semikhatsky]

Created attachment 60691 [details]
Patch

[Pavel Feldman]

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]

Created attachment 60706 [details]
Patch

[Yury Semikhatsky]

Created attachment 60709 [details]
Patch

[Yury Semikhatsky]

Addressed all comments. Please take a look.

[Yury Semikhatsky]

Committed r62647