Closed Bug 865688 Opened 7 years ago Closed 7 years ago

Style Inspector sidebar should remember its size

Categories

(DevTools :: Inspector, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 24

People

(Reporter: rcampbell, Assigned: paul)

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(2 files)

When reopening the inspector, it should remember the width of the style sidebar.
Assignee: nobody → paul
Attached patch Patch v1Splinter Review
Attachment #760393 - Flags: review?(mihai.sucan)
This patch saves the width for scratchpad, the webconsole and the inspector
Comment on attachment 760393 [details] [diff] [review]
Patch v1

Review of attachment 760393 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Thank you!

::: browser/devtools/framework/sidebar.js
@@ +5,5 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +const {Cu} = require("chrome");
> +
> +Cu.import("resource://gre/modules/Services.jsm");

Should we use addon-sdk stuff here?

See https://addons.mozilla.org/en-US/developers/docs/sdk/latest/modules/sdk/simple-prefs.html

@@ +36,5 @@
>    this._panelDoc = this._tabbox.ownerDocument;
>    this._toolPanel = panel;
>  
> +  try {
> +    this._width = Services.prefs.getIntPref("devtools.toolsidebar-width." + this._uid);

This pref name prefix is repeated several times in this file. Maybe a const at the top of the file makes sense.
Attachment #760393 - Flags: review?(mihai.sucan) → review+
(In reply to Mihai Sucan [:msucan] from comment #3)
> Comment on attachment 760393 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 760393 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good. Thank you!
> 
> ::: browser/devtools/framework/sidebar.js
> @@ +5,5 @@
> >   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> >  
> > +const {Cu} = require("chrome");
> > +
> > +Cu.import("resource://gre/modules/Services.jsm");
> 
> Should we use addon-sdk stuff here?
> 
> See
> https://addons.mozilla.org/en-US/developers/docs/sdk/latest/modules/sdk/
> simple-prefs.html

I think this does a little too much (exposing UI element in the pref panel).
 
> @@ +36,5 @@
> >    this._panelDoc = this._tabbox.ownerDocument;
> >    this._toolPanel = panel;
> >  
> > +  try {
> > +    this._width = Services.prefs.getIntPref("devtools.toolsidebar-width." + this._uid);
> 
> This pref name prefix is repeated several times in this file. Maybe a const
> at the top of the file makes sense.

ok.
Attached patch Patch v1.1Splinter Review
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/de9bcd3c8e6c
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/de9bcd3c8e6c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → Firefox 24
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.