Cannot suspend web-ext

[andymckay]

Is this a bug or feature request?

Bug

What is the current behavior?

web-ext run

Then press ctrl-z and expect the process to be suspended so you can resume later. It does not.

What is the expected or desired behavior?

ctrl-z suspends the information.

Version information (for bug reports)

• Firefox version: Nightly
• Your OS and version: OS X 10.10.5
• Paste the output of these commands:

node --version && npm --version && web-ext --version

v7.8.0
4.2.0
1.9.0

This used to work, might have been a result of the handling for "Press R to reload".

[kumar303]

I tried a version of web-ext before and after #705 and I can confirm that after #705, suspending is not working (with Node 6.10.2).

If I'm not mistaken, bash sends SIGSTOP when you type ctrl-z. Perhaps the co-routine running the while loop needs to look for this somehow? @saintsebastian @rpl any ideas on how to fix this?

[saintsebastian]

Tbh I didn't know about this behaviour and completely ignored it while writing that feature. Sorry about that. Don't see reasons why loop can't expect ctrl-z as and send SIGSTOP. Have to test it first though.

[kumar303]

It's possible Node is getting the SIGTSTP signal instead of SIGSTOP, I'm not sure.

[saintsebastian]

@kumar303 i think it was SIGTSTP. I can definitely send it to process, the more interesting question is how one can restart the loop on fg. I will think it other and try to come up with some solution and start a PR.

[rpl]

@andymckay @kumar303 yeah, it looks like a side-effect of enabling the readline keypress events on the stdin stream, we handled it explicitly for the Ctrl-C keyboard shortcut (https://github.com/mozilla/web-ext/blob/master/src/cmd/run.js#L156-L157), but we didn't for Ctrl-Z.

@saintsebastian don't worry about how to restore the process with fg: it is something that the shell application will take care of.

The only thing that we have to do to restore the behavior of Ctrl-Z is to do it explicitly with process.kill(process.pid, 'SIGTSTP') (process.pid will return the pid of the current nodejs process)

e.g. the following patch should be enough to fix it where SIGTSTP is supported (e.g. Linux and OSX):

diff --git a/src/cmd/run.js b/src/cmd/run.js
index 74510e5..402ff56 100644
--- a/src/cmd/run.js
+++ b/src/cmd/run.js
@@ -155,6 +155,8 @@ export function defaultReloadStrategy(

         if (keyPressed.ctrl && keyPressed.name === 'c') {
           userExit = true;
+        } else if (keyPressed.ctrl && keyPressed.name === 'z') {
+          process.kill(process.pid, 'SIGTSTP');
         } else if (keyPressed.name === 'r' && addonId) {
           log.debug('Reloading extension on user request');
           await addonReload({addonId, client});

[saintsebastian]

@rpl oh I tested this yesterday and it didn't work on my environment. Interesting

[rpl]

@saintsebastian that's weird, I just tried on OSX (on a OS X 10.10 Yosemite installation) and it worked as expected, on which version of OSX/node/npm you tested it?

[rpl]

@saintsebastian How about creating a new pull request with the change that you are trying locally?

it would help to figure out if we are testing the same change (and I can test your change directly on Linux and OSX to check why it is not working as expected on your system).

[shubheksha]

Can this be closed as #948 is merged?

[rpl]

@shubheksha yep, we can definitely close this now.