fix: restore stdin raw mode to off after ctrl-c quit

[qupig]

When using web-ext via API, after quit via Ctrl-C, the parent process can no longer respond to any keyboard input.

This PR fixes the issue by correctly returning stdin to the parent process.

This fix is ​​only tested and confirmed in the firefox-desktop version.
The firefox-android process may have similar issue, but I cannot test it at the moment.

[qupig]

The CI error depends on the dependency update:

• chore(deps): bump addons-linter from 7.9.0 to 7.10.0 #3393
• chore(deps): bump addons-linter from 7.9.0 to 7.11.0 #3396

[rpl]

@qupig would be possible from the nodejs app using web-ext as a dependency to do process.stdin.destroy() call on its side or the API as exposed by web-ext not allowing that? (e.g. missing a callback or some other hook point to do that kind of cleanup)?

[qupig]

@rpl To be honest, I haven't carefully studied the process stack and architecture of web-ext.

In nodejs process we call await webExt.cmd.run(), then, stdin is caught by web-ext and we can't tell when web-ext ends, like you said, I don't see a callback that can be called.

But I think the more important problem is that it doesn't should to be cleaned up manually with some kind of callback, doesn't it? After the web-ext ends, it should release the resource correctly, and then the parent program is able to respond correctly to keyboard input.

I'm not sure this changes is the most correct way, but it does work back to normal in my tests.

I think it would be nice to have a callback when web-ext exit, but this stdin cleanup should seem to be automatic than manually.

[qupig]

I just looked at the code logic a little again, and it seems that it would be more appropriate to modify it here:

web-ext/src/extension-runners/index.js

Lines 255 to 260 in b85dbe4

	extensionRunner.registerCleanup(() => {
	watcher.close();
	if (allowInput) {
	stdin.pause();
	}
	});

-     stdin.pause();
+     stdin.destroy();

I'm not sure what the original stdin.pause(); does, but it won't release stdin correctly.

[Rob--W]

If you use it as a library, would the noInput option better match your needs? Then web-ext won't capture the stdin.

[qupig]

@Rob--W

If you use it as a library, would the noInput option better match your needs? Then web-ext won't capture the stdin.

Yeah, I tried this, but we lost the shortcut key R to reload extension, also impossible to stop only web-ext instead of process via Ctrl-C.

UPDATE: Sorry I rechecked the code and await webExt.cmd.run() returned extensionRunner, so I can actually write additional code to listen for these shortcuts by calling extensionRunner.reloadAllExtensions() and extensionRunner.exit().

But still, is it better practice to consider releasing stdin correctly in web-ext?
I mean after we exit web-ext with Ctrl-C, there seems to be no reason not to release stdin there, right?

[qupig]

So I didn't realize before that I could actually register a callback to clean it up:

const extensionRunner = await webExt.cmd.run(/*...*/);
extensionRunner.registerCleanup(() => {
	process.stdin.destroy();
});

But I still think this cleanup should be the default behavior unless there are obvious reasons or side effects there.

Anyway, thanks for the info you guys provided! If you think this is not necessary to fix it, feel free to close it.

[qupig]

Further investigation revealed that it wasn't directly related to the stdin release, it just happened to fix the issue.
The real issue was that web-ext set setRawMode(stdin, true); and did not restore it on exit.

I updated the PR and tested that it works correctly for me. Please re-review if this PR should be merged.