Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ctrl + C does nothing when using firefox-android #1699

Open
SunriseFox opened this issue Sep 6, 2019 · 11 comments · May be fixed by #2523
Open

Ctrl + C does nothing when using firefox-android #1699

SunriseFox opened this issue Sep 6, 2019 · 11 comments · May be fixed by #2523

Comments

@SunriseFox
Copy link

Is this a feature request or a bug?

Bug.

What is the current behavior?

Ctrl + C does nothing.

What is the expected or desired behavior?

Should quit the program.

Version information (for bug reports)

  • Firefox version: -
  • Your OS and version: Windows
  • web-ext: 3.1.1

The very likely bad codes:
src\extension-runners\firefox-android.js:537

    const handleCtrlC = (str, key) => {
      if (key.ctrl && key.name === 'c') {
        adbUtils.setUserAbortDiscovery(true);
      }
    };

    // TODO: use noInput property to decide if we should
    // disable direct keypress handling.
    if (isTTY(stdin)) {
      readline.emitKeypressEvents(stdin);
      setRawMode(stdin, true);

      stdin.on('keypress', handleCtrlC);
    }

#1569

@rpl
Copy link
Member

rpl commented Sep 26, 2019

@SunriseFox we have a couple of questions to get a better picture of the issue (and how to reproduce it):

  • is the Firefox for android instance already running while you are pressing Ctrl-C?
  • which windows shell are you using? (e.g. cmd or powershell)
  • are you using any third party terminal application or the stardard one?

@SunriseFox
Copy link
Author

SunriseFox commented Sep 26, 2019

  1. Yes.
  2. Powershell.
  3. The standard one.

@CS-Aditya-Rawat
Copy link

@SunriseFox Can I start work on this?

@rpl
Copy link
Member

rpl commented Jan 11, 2021

Can I start work on this?

CS-Aditya-Rawat This bug requires some more investigation to determine more details about the underlying issue (and based on that the kind of changes we would prefer to fix it as its first step), and so if you are looking for a good-first-bug as a initial contribution then this one may not be a good "entry point" (at least not yet).

If a "good first bug" is what you are looking for, you may take a look to the ones not yet assigned to a contributor (or a PR recently open and linked to the issue) from this list:

Or one from the "contrib:welcome" list (but be aware that the older issues in the "contrib:welcome" list may not be relevant anymore but not closed yet, and they may require a bit more experience than the one marked as "good first bug"):

@luskaner
Copy link

luskaner commented Oct 5, 2022

Hello:

I've investigated this issue, and the problem arises in this finally block

with the whole relevant section being:

 try {
      // Got a debugger socket file to connect.
      this.selectedRDPSocketFile = (
        await adbUtils.discoverRDPUnixSocket(
          selectedAdbDevice, selectedFirefoxApk, {
            maxDiscoveryTime: unixSocketDiscoveryMaxTime,
            retryInterval: unixSocketDiscoveryRetryInterval,
          }
        )
      );
    } finally {
      if (isTTY(stdin)) {
        stdin.removeListener('keypress', handleCtrlC);
      }
    }

Removing the finally block (or the whole try...finally block leaving the try code for that matter) fixes the issue.
Honestly, I don't understand why is that finally there to begin with as you are always removing the listener regardless, maybe it was meant to be a catch?

@luskaner
Copy link

luskaner commented Oct 7, 2022

@rpl can you assign me to this? This would be my first contribution but I think it's a pretty easy fix

@willdurand
Copy link
Member

@luskaner feel free to submit a PR with "Fixes #1699" in the description, we'll assign you then

@luskaner
Copy link

luskaner commented Oct 7, 2022

@willdurand thanks will do

@luskaner
Copy link

luskaner commented Oct 7, 2022

Further investigating this happens in either of the following two conditions (in run):

  • Automatic reloading is disabled.
  • Waiting to launch the activity on Android. For example: this occurs undefinitely when my device when it is locked.

@luskaner
Copy link

@willdurand PR linked and ready ;)

@willdurand
Copy link
Member

(I didn't forget about this PR, just have no time at the moment, sorry - I'll get back to it ASAP)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants