1472309 - Don't enable the 3 pane inspector by default on the browser toolbox

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Comment 1

[Gabriel Luong [:gl] (ΦωΦ)]

The idea is to add a separate pref to control the inspector 3 pane mode for the browser toolbox (chrome). Typically, gecko developers are opening the browser toolbox in a split screen situation so their screen size is already less than ideal for the 3 pane mode which can take advantage of the full length of the screen.

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1472309 - Don't enable the 3 pane inspector by default on the browser toolbox.

Review commit: https://reviewboard.mozilla.org/r/254108/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/254108/

Comment 3

[Gabriel Luong [:gl] (ΦωΦ)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=70a90b8a9cbd4e236214c11558b190c89ccd4292

Comment 4

[Patrick Brosset <:pbro>]

Comment on attachment 8989021 [details]
Bug 1472309 - Don't enable the 3 pane inspector by default on the browser toolbox.

https://reviewboard.mozilla.org/r/254108/#review260900

Comment 5

[Patrick Brosset <:pbro>]

Weird, I made a comment on mozreview, but an error occurred and that comment is now lost. I'll try to recover it from mozreview.

Comment 6

[Patrick Brosset <:pbro>]

Comment on attachment 8989021 [details]
Bug 1472309 - Don't enable the 3 pane inspector by default on the browser toolbox.

https://reviewboard.mozilla.org/r/254108/#review260904

::: devtools/client/inspector/inspector.js:693
(Diff revision 1)
> -    this.is3PaneModeEnabled = !this.is3PaneModeEnabled;
> -    Services.prefs.setBoolPref(THREE_PANE_ENABLED_PREF, this.is3PaneModeEnabled);
> +    if (this.target.chrome) {
> +      this._is3PaneModeChromeEnabled = !this._is3PaneModeChromeEnabled;
> +      Services.prefs.setBoolPref(THREE_PANE_CHROME_ENABLED_PREF,
> +        this._is3PaneModeChromeEnabled);
> +    } else {
> +      this._is3PaneModeEnabled = !this._is3PaneModeEnabled;
> +      Services.prefs.setBoolPref(THREE_PANE_ENABLED_PREF, this._is3PaneModeEnabled);
> +    }

Could you please move this code to a setter, next to the getter you created?

```
set is3PaneModeEnabled(value) {
  ...
}
```

And rewrite this function to:

```
async onSidebarToggle() {
  this.is3PaneModeEnabled = !this.is3PaneModeEnabled;
  await this.setupeToolbar();
  await this.addRuleView({ skipQueue: true });
}
```

The goal is to keep the complexity of reading the prefs and setting these private properties hidden away in the getter/setter pair.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea033f144afe
Don't enable the 3 pane inspector by default on the browser toolbox. r=pbro

Comment 8

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/ea033f144afe