Closed Bug 1445776 Opened 6 years ago Closed 6 years ago

Add browser restart shortcut to Browser Console window (development only)

Categories

(DevTools :: Console, enhancement, P3)

enhancement

Tracking

(firefox61 fixed)

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: jryans, Assigned: jryans)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The main browser window now has a restart shortcut for local development, which is quite nice.

I commonly also open the Browser Console window.  When I switch to the app to refresh, I sometimes get the Browser Console window focused first, and so it would be nice for the restart shortcut to also work in that window.

I don't think it's necessary to also show the menu item.
(I'll do this for the new frontend only.)
Comment on attachment 8958993 [details]
Bug 1445776 - Add development restart shortcut to Browser Console.

https://reviewboard.mozilla.org/r/227852/#review233688

::: devtools/client/webconsole/new-webconsole.js:260
(Diff revision 1)
>        ZoomKeys.register(this.window);
> +
> +      if (!system.constants.MOZILLA_OFFICIAL) {
> +        if (system.constants.platform === "macosx") {
> +          // On macOS, the `key` property is `®` instead of `r`
> +          shortcuts.on("CmdOrCtrl+Alt+®", this.window.DevelopmentHelpers.quickRestart);

Weird. Can you add this condition into key-shortcuts.js instead? That would be sure to save the next person who needs to use this shortcut some time
Comment on attachment 8958993 [details]
Bug 1445776 - Add development restart shortcut to Browser Console.

https://reviewboard.mozilla.org/r/227852/#review233694

::: devtools/client/webconsole/new-webconsole.js:258
(Diff revision 2)
>                     this.window.top.close.bind(this.window.top));
>  
>        ZoomKeys.register(this.window);
> +
> +      if (!system.constants.MOZILLA_OFFICIAL) {
> +        shortcuts.on("CmdOrCtrl+Alt+R", this.window.DevelopmentHelpers.quickRestart);

I know I was originally the one who suggested loading this as a script tag but after playing around with it locally, I think I'd prefer something like:

1) Remove ChromeUtils.import call from browser-development-helpers.js
2) Remove script tag from webconsole.html
3) Import it inside the condition as something like:

```
// In local builds, inject the 'quick restart' shortcut. This script expects to have
// Services on the global and we haven't yet imported it into the window, so assign it.
this.window.Services = Services;
Services.scriptloader.loadSubScript("chrome://browser/content/browser-development-helpers.js", this.window);
shortcuts.on("CmdOrCtrl+Alt+R", this.window.DevelopmentHelpers.quickRestart);
```

This way we don't load the extra script unless if we are in dev builds, and we don't need to make a change to the dev-helpers file which could get broken during an unrelated refactor.

This should eventually get simpler as well once we do Bug 1442006 or follow ups to it and introduce "Services" as a global in all chrome windows.

::: devtools/client/webconsole/new-webconsole.js:258
(Diff revision 2)
>                     this.window.top.close.bind(this.window.top));
>  
>        ZoomKeys.register(this.window);
> +
> +      if (!system.constants.MOZILLA_OFFICIAL) {
> +        shortcuts.on("CmdOrCtrl+Alt+R", this.window.DevelopmentHelpers.quickRestart);

I know I was originally the one who suggested loading this as a script tag but after playing around with it locally, I think I'd prefer something like:

1) Remove ChromeUtils.import call from browser-development-helpers.js
2) Remove script tag from webconsole.html
3) Import it inside the condition as something like:

```
// In local builds, inject the 'quick restart' shortcut. This script expects to have
// Services on the global and we haven't yet imported it into the window, so assign it.
this.window.Services = Services;
Services.scriptloader.loadSubScript("chrome://browser/content/browser-development-helpers.js", this.window);
shortcuts.on("CmdOrCtrl+Alt+R", this.window.DevelopmentHelpers.quickRestart);
```

This way we don't load the extra script unless if we are in dev builds, and we don't need to make a change to the dev-helpers file which could get broken during an unrelated refactor.

This should eventually get simpler as well once we do Bug 1442006 or follow ups to it and introduce "Services" as a global in all chrome windows.
Attachment #8958993 - Flags: review?(bgrinstead)
Comment on attachment 8958993 [details]
Bug 1445776 - Add development restart shortcut to Browser Console.

https://reviewboard.mozilla.org/r/227852/#review233708

Thanks
Attachment #8958993 - Flags: review?(bgrinstead) → review+
Pushed by jryans@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e4df3b09cb1a
Add development restart shortcut to Browser Console. r=bgrins
https://hg.mozilla.org/mozilla-central/rev/e4df3b09cb1a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: