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

Can't run with specified profile name and --keep-profile-changes #932

Comments

@eight04
Copy link
Contributor

eight04 commented Apr 30, 2017

Is this a bug or feature request?

Bug.

What is the current behavior?

D:\Dev\in-my-pocket> web-ext run -s build -p test --keep-profile-changes --verbose

[src\program.js][debug] Getting version from the git revision
[src\program.js][info] Version: master-36cc19fe3a71d912296fdc458033091633fcf0e7
[src\cmd\run.js][info] Running web extension from D:\Dev\in-my-pocket\build
[src\util\manifest.js][debug] Validating manifest at D:\Dev\in-my-pocket\build\manifest.json
[src\cmd\run.js][debug] Using Firefox profile from test
[src\program.js][error] 
run: Error: ENOENT: no such file or directory, open 'D:\Dev\in-my-pocket\test\user.js'
    at Object.fs.openSync (fs.js:584:18)
    at Object.fs.writeFileSync (fs.js:1316:33)
    at FirefoxProfile._writeUserPrefs (D:\Dev\web-ext\node_modules\firefox-profile\lib\firefox_profile.js:517:6)
    at FirefoxProfile.updatePreferences (D:\Dev\web-ext\node_modules\firefox-profile\lib\firefox_profile.js:347:8)
    at configureProfile (D:\Dev\web-ext\dist\webpack:\src\firefox\index.js:227:11)
    at Object._callee3$ (D:\Dev\web-ext\dist\webpack:\src\firefox\index.js:250:16)
    at tryCatch (D:\Dev\web-ext\node_modules\regenerator-runtime\runtime.js:65:40)
    at Generator.invoke [as _invoke] (D:\Dev\web-ext\node_modules\regenerator-runtime\runtime.js:303:22)
    at Generator.prototype.(anonymous function) [as next] (D:\Dev\web-ext\node_modules\regenerator-runtime\runtime.js:117:21)
    at step (D:\Dev\web-ext\dist\webpack:\~\babel-runtime\helpers\asyncToGenerator.js:17:1)

[src\program.js][error] run: Error code: ENOENT

However, both

web-ext run -s build -p test
web-ext run -s build -p path/to/test/profile --keep-profile-changes

work fine.

What is the expected or desired behavior?

I think it should convert the profile name to file path.

Version information (for bug reports)

  • Firefox version: 54.a2
  • Your OS and version: Windows 7 x64
  • Paste the output of these commands:
node --version && npm --version && web-ext --version
D:\Dev\in-my-pocket>node --version && npm --version && web-ext --version
v7.9.0
4.6.1
master-36cc19fe3a71d912296fdc458033091633fcf0e7
@rpl rpl added the type: bug label May 2, 2017
@rpl
Copy link
Member

rpl commented May 2, 2017

Currently, if the --keep-profile-changes options has been enabled, we are calling firefoxApp.useProfile instead of firefoxApp.copyProfile.

firefoxApp.copyProfile handles internally both the scenarios (a profile passed by name and by path):

  • if (dirExists) {
    log.debug(`Copying profile directory from "${profileDirectory}"`);
    profile = await copy({profileDirectory});
    } else {
    log.debug(`Assuming ${profileDirectory} is a named profile`);
    profile = await copyByName({name: profileDirectory});
    }

while in firefoxApp.useProfile we are currently handling only the scenario where the profile is a path:

  • const profile = new FirefoxProfile({destinationDirectory: profilePath});
    return await configureThisProfile(profile, {app, customPrefs});

To fix this issue we can choose between a couple of different strategies:

  • explicitly require a profile path with --keep-profile-changes, but raise a more helpful UserError when the profile is not an existent path

  • or copy the profile by name when the profile passed is not an existent path, but do not remove the cloned profile when web-ext exits (because of the --keep-profile-changes option)

  • or resolve the profile name into a profile dir and use that profile (which is probably a bit more risky because the user can end up making changes to a "production" Firefox profile by mistake)

@kumar303 @saintsebastian what do you think about the three strategies describe above?

@kumar303
Copy link
Contributor

kumar303 commented May 2, 2017

I think we should solve it like this:

...resolve the profile name into a profile dir and use that profile (which is probably a bit more risky because the user can end up making changes to a "production" Firefox profile by mistake)

This seems like the right thing to do. It's not any more risky than --keep-profile-changes already is. When you use this option you are already accepting the risk of making destructive changes to the profile. I made sure the docs warn about this: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Getting_started_with_web-ext#Keeping_profile_changes

@rpl
Copy link
Member

rpl commented May 2, 2017

This seems like the right thing to do. It's not any more risky than --keep-profile-changes already is.

@kumar303 How about preventing the user to select default and default-dev-edition by profile name? (these two are the most risky ones, e.g. because we enable connections to the remote debugging without any confirmation dialog in the selected profile)

They would be still be selectable using the full profile path, but that it is not implicit (and easy to pass by mistake) as using the profile name.

How that sounds to you?

Besides the above concern, it sounds good to me.
From an implementation point of view, it looks like resolving the profile name into a profile path is not directly exposed (at least in the current version) as an exported helper by the firefox-profile dependency, but it exports FirefoxProfile.Finder which is the helper used internally:

@kumar303
Copy link
Contributor

kumar303 commented May 2, 2017

How about preventing the user to select default and default-dev-edition by profile name?

Yeah, that's wise. Let's add that check too.

@shubheksha
Copy link
Contributor

@harikishen is working on this in #968

@mlissner
Copy link

mlissner commented Dec 4, 2017

Looks like the PR for this is totally frozen as of June or so.

I'm going to update the documentation to point to this bug and state that the feature doesn't work. Is there any workaround at all that should be mentioned there or is --keep-profile-changes totally broken (as it appears to be)?

@kumar303
Copy link
Contributor

kumar303 commented Dec 4, 2017

I'm going to update the documentation to point to this bug and state that the feature doesn't work.

It works fine for existing profiles.

Is there any workaround at all that should be mentioned there or is --keep-profile-changes totally broken (as it appears to be)?

Maybe you intended to file a different bug rather than comment on this one? The workaround to #932 is to create the profile first and then re-run the web-ext command with --keep-profile-changes.

@mlissner
Copy link

mlissner commented Dec 4, 2017

Oh...um, that's not working for me. I just got:

↪ web-ext run --firefox-profile 57 --keep-profile-changes 
Running web extension from /home/mlissner/Programming/intellij/recap-chrome

run: Error: ENOENT: no such file or directory, open '57/user.js'
    at Object.fs.openSync (fs.js:653:18)
    at Object.fs.writeFileSync (fs.js:1300:33)
    at FirefoxProfile._writeUserPrefs (/usr/local/lib/node_modules/web-ext/node_modules/firefox-profile/lib/firefox_profile.js:517:6)
    at FirefoxProfile.updatePreferences (/usr/local/lib/node_modules/web-ext/node_modules/firefox-profile/lib/firefox_profile.js:347:8)
    at configureProfile (/usr/local/lib/node_modules/web-ext/dist/webpack:/src/firefox/index.js:9:37760)
    at Object._callee4$ (/usr/local/lib/node_modules/web-ext/dist/webpack:/src/firefox/index.js:9:20121)
    at tryCatch (/usr/local/lib/node_modules/web-ext/node_modules/regenerator-runtime/runtime.js:65:40)
    at Generator.invoke [as _invoke] (/usr/local/lib/node_modules/web-ext/node_modules/regenerator-runtime/runtime.js:303:22)
    at Generator.prototype.(anonymous function) [as next] (/usr/local/lib/node_modules/web-ext/node_modules/regenerator-runtime/runtime.js:117:21)
    at step (/usr/local/lib/node_modules/web-ext/dist/webpack:/~/babel-runtime/helpers/asyncToGenerator.js:17:1)

run: Error code: ENOENT

And that profile exists:

screenshot from 2017-12-04 13-29-28

@kumar303
Copy link
Contributor

kumar303 commented Dec 4, 2017

Did you create 57 with the profile manager? (I just saw your screenshot, thanks)

Also, does this work?

web-ext run --firefox-profile path/to/your/created/profile/57 --keep-profile-changes

@mlissner
Copy link

mlissner commented Dec 4, 2017

Providing the full path does work, so that's a workaround. Do we have enough to update the docs?

@kumar303
Copy link
Contributor

kumar303 commented Dec 4, 2017

Well, I'd rather get the issue fixed. If someone encounters it they will probably search the error and find this issue which documents a workaround.

@mlissner
Copy link

mlissner commented Dec 4, 2017

We're six months along with no fix. I suspect the fix is documentation.

@saintsebastian
Copy link
Contributor

I wrote the original --keep-profile-changes feature, so maybe with the help of work done by @harikishen I can move this forward relatively fast. Should I work on the same branch and just go on with existing PR? I'll pick it up before the end of the week.

@kumar303
Copy link
Contributor

kumar303 commented Dec 5, 2017

Thanks so much @saintsebastian . It's probably easiest to create a new PR, whatever works. Also, I have your config patch in my review queue, just a little backlogged 🤐

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment