-
Notifications
You must be signed in to change notification settings - Fork 31
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
support ubuntu path (~/snap/...) #139
Conversation
lib/profile_finder.js
Outdated
@@ -16,6 +16,14 @@ function locateUserDirectory(platform) { | |||
break; | |||
case 'linux': | |||
userDir = path.join(process.env.HOME, '/.mozilla/firefox'); | |||
// fix: https://github.com/saadtazi/firefox-profile-js/issues/136 | |||
// check if directory is present, otherwise use ubuntu path: ~/snap/firefox/common/.mozilla/firefox/ | |||
if (!fs.existsSync(userDir)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested my code yet, but it was along the same lines:
snapDir = ...
dotDir = ...
xdgConfigDir = process.env.XDG_CONFIG_HOME || path.join(process.env.HOME, ".config")
xdgDir = path.join(xdgConfigDir, "mozilla/firefox")
if (fs.existsSync(dotDir)) { userDir = dotDir }
else if (fs.existsSync(snapDir)) { userDir = snapDir }
else { userDir = xdgDir }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I wasn't sure what the XDG_CONFIG_HOME was used for, thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed some changes, it's now closer to the logic you proposed.
Mentioning the issue for easier access: #136 |
This may be a dumb question, and totally unrelated to the issue, but is there any reason in particular to use |
f784362
to
f601e79
Compare
good question: I no longer use this package, so I didn't take the time to update the code (nor the test/CI tools) |
On second thought, I think In this case, I think the best behavior would be to assume that we can be dealing with a sandboxed or unsandboxed firefox install, where the normal home can still be a valid location for profile data, since both snap and flatpak seem to allow it. So the code could be something like: function locateLinuxDirectory() {
userHomeDir = process.env.HOME;
// look for a valid sandboxed profile dir
homes = [
path.join(userHomeDir, "snap/firefox/common"),
path.join(userHomeDir, ".var/app/org.mozilla.firefox")
]
for (home of homes) {
dotDir = path.join(home, ".mozilla/firefox")
xdgDir = path.join(home, ".config/mozilla/firefox")
if (fs.existsSync(dotDir) {
return dotDir
} else if (fs.existsSync(xdgDir)) {
return xdgDir
}
}
// look in the next probable locations
dotDir = path.join(userHomeDir, ".mozilla/firefox")
xdgConfigDir = process.env.XDG_CONFIG_HOME || path.join(userHomedir, ".config")
xdgDir = path.join(xdgConfigDir, "mozilla/firefox")
return fs.existsSync(dotDir) ? dotDir : xdgDir;
} |
f601e79
to
0056d73
Compare
sorry for the delay, I didn't have access to a computer... pushed some changes... |
I think the logic is still inverted for global installs. The dot dir should be preferred if it exists, but it should not be the default directory. Also, the XDG config dir doesn't need a leading dot. |
@@ -5,23 +5,34 @@ var os = require('os'), | |||
path = require('path'), | |||
ini = require('ini'); | |||
|
|||
function locateLinuxDirectory() { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, the second corner case would actually mean that something is terribly wrong or Firefox is not installed, so maybe throwing an error would be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am really sorry for the delay, got a new job last week, I didn't had time to finish this PR...
To prevent breaking changes (for now...), I added a warning if no linux folder is found, but it continues (no error thrown). I will publish a new version with current changes.
if linux, check if default firefox/ dir is present, if not, use ubuntu path.