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

prefsCleaner.sh (for Linux/Mac) #405

Merged
merged 11 commits into from Apr 24, 2018
Merged

prefsCleaner.sh (for Linux/Mac) #405

merged 11 commits into from Apr 24, 2018

Conversation

claustromaniac
Copy link
Contributor

@claustromaniac claustromaniac commented Apr 20, 2018

Long overdue port of the prefsCleaner.bat for Unix-like systems. I was bored and had the time for it so.. why not?

This is my very first shell script ever, and I have only tested it with a minimal Cygwin install, so all feedback is very welcome. I did my best aiming for compatibility and reliability but, being a beginner and all, it would surprise me if this couldn't be improved still.

Bear in mind that I couldn't let go of bash-only syntax completely, so this won't work in other shells.

@1xPdd, I hope you're still up for testing.

Download link for the lazy ones. (the PR was merged, grab the one in this repository instead)

Port of the prefsCleaner.bat for anything(?) with a bash shell. First functional commit tested on a minimal Cygwin install.
@Thorin-Oakenpants
Copy link
Contributor

Make sure you let CHEF-KOCH know, so he can plagiarize it

@overdodactyl
Copy link
Contributor

Really appreciate this @claustromaniac!

I did a quick test (nothing extensive) and it seems to work great!

My only comment/suggestion after a quick run would be about the shebang. Instead of #!/usr/bin/env I think you might want to use #!/usr/bin/env bash.

When using the first, I noticed two "problems" on macOS (neither of huge concern, admittedly):

  • You have to specify sh in the terminal shell when running. In other words, you can't just drag/drop the file as you normally can. Without including sh, the script won't run.

  • When you do use sh, printing is a little off (-e's show up from echo commands):

-e 
                   ╔══════════════════════════╗
                   ║     prefs.js cleaner     ║
                   ║    by claustromaniac     ║
                   ║          v1.0b           ║
                   ╚══════════════════════════╝
-e 
This script should be run from your Firefox profile directory.

It will remove any entries from prefs.js that also exist in user.js.
This will allow inactive preferences to be reset to their default values.
-e 
This Firefox profile shouldn't be in use during the process.

@claustromaniac
Copy link
Contributor Author

Thanks @overdodactyl. I had my share of doubts about the shebang. I'll change it right away :)

@overdodactyl
Copy link
Contributor

Little off topic here (I can open a new issue if that's preferable), but what's your guy's opinion on integrating the cleaners and updaters through some means? Couple possibilities would be to combine into one script, another would be for the updater to call/execute the cleaner script after it updates to the latest user.js.

@claustromaniac
Copy link
Contributor Author

For me it would be nice if the updater could detect the presence of the cleaner and call it. Maybe with a prompt after the updating is over.

@claustromaniac
Copy link
Contributor Author

Or better yet, a prompt before the updating even begins.

@overdodactyl
Copy link
Contributor

Before would be ideal I think!

Might be nice to show the prompt even if it isn't present...if that's the case, the script would download the cleaner script, make it executable and eventually run it.

Similar idea to the code lines here:

https://github.com/overdodactyl/ShadowFox/blob/master/scripts/ShadowFox_updater_linux.sh#L113-L129

If you think there might be frequent updates to the cleaner script, the updater could also run a check for version number and install the latest one if necessary. Similar to lines here:

https://github.com/overdodactyl/ShadowFox/blob/master/scripts/ShadowFox_updater_mac.sh#L50-L67

@claustromaniac
Copy link
Contributor Author

Might be nice to show the prompt even if it isn't present...if that's the case, the script would download the cleaner script, make it executable and eventually run it.

If someone doesn't want to use the cleaner for some reason they would get that prompt continuously, unless we managed to make it happen only once somehow (like with a config file or something). I think I would rather add a message in the updater informing users that it can now launch the cleaner if it's present. It could even be a read-this-while-you-wait sort of message, to make it even less intrusive.

If you think there might be frequent updates to the cleaner script, the updater could also run a check for version number and install the latest one if necessary.

Frankly, I tried my best to make both this and the windows version of the cleaner as complete and robust as I could from the beginning so I wouldn't have to push many updates in the future, but I don't think that's a bad idea at all. It could be a worthy addition, or at least it wouldn't hurt.

@overdodactyl
Copy link
Contributor

If someone doesn't want to use the cleaner for some reason they would get that prompt continuously,

Would definitely be a bother to be asked each time, that's a good point. I do wonder if there are many use cases where one wouldn't want to use the cleaner though?

@claustromaniac
Copy link
Contributor Author

claustromaniac commented Apr 21, 2018

I do wonder if there are many use cases where one wouldn't want to use the cleaner though?

You and me both. But if such use cases exist, I won't want to be pushy :)

@earthlng
Copy link
Contributor

earthlng commented Apr 21, 2018

For me it would be nice if the updater could detect the presence of the cleaner and call it. Maybe with a prompt after the updating is over.

Or better yet, a prompt before the updating even begins.

I like the idea but why would before be better than after?

Might be nice to show the prompt even if it isn't present...if that's the case, the script would download the cleaner script, make it executable and eventually run it.

IMO it would be nice to prompt if the cleaner is present and maybe show a message if it isn't. Something like "You may want to reset deprecated/removed prefs now. Check out our prefsCleaner scripts." with a link to 1 of our wiki pages.
Users need to understand the pros and cons of the prefsCleaner and making it too easy to download + run it might not be the best idea.
For example, somebody running FF Beta could have already flipped a "new" pref in about:config that we didn't have in the user.js at that time. Now we've added it, but inactive - the prefsCleaner will remove that pref from the users prefs.js and (s)he might not notice it.

@claustromaniac
Copy link
Contributor Author

claustromaniac commented Apr 21, 2018

I like the idea but why would before be better than after?

It wouldn't be better by itself. I was thinking that if we added a prompt like "Would you like to run the prefsCleaner afterwards?" and I added a parameter to the cleaners so they can run unattended, the users would then be allowed to walk away after all those prompts and leave the thing doing its business.

Neither the updaters nor the cleaners take long to run, though, so it might not be worth it. IDK. It was just a thought.

@earthlng
Copy link
Contributor

Oh okay, yeah that makes sense.

@earthlng
Copy link
Contributor

earthlng commented Apr 22, 2018

  • $bakfile in fClean $bakfile and the $1 in done < $1 > prefs.js should probably be wrapped in quotes, just in case
  • can you refresh my memory what the difference is between done < and done <<< ?
  • not sure but I think the way you look for string-in-string is kinda risky. There are no separators between prefs, right? For example looking for a.b.c would match if there's a different pref a.b.c.d AFAICS, no?
  • after having added separators, shouldn't *${BASH_REMATCH[1]}* also be wrapped in quotes, just in case? not sure exactly how the quotes need to be set but I'd imagine something like *@@"${BASH_REMATCH[1]}"@@*. So you would start with prefs="@@" and also append it in prefs=${prefs}${BASH_REMATCH[1]}@@. IDK if @ is a good separator though. You get the idea, I'm sure. These are all just example and I have no idea if the format is correct so please don't copy it without testing :)

@earthlng
Copy link
Contributor

earthlng commented Apr 22, 2018

https://stackoverflow.com/questions/229551/string-contains-in-bash

if [[ $string =~ .*My.* ]]

The =~ operator already searches the whole string for a match; the .*'s here are extraneous.

^^ 87 upvotes. I know you're currently using != instead of =~ but ...

Test if it does NOT contain a string: if [[ ! "abc" =~ "d" ]]

this way you don't need the asterisks around ${BASH_REMATCH[1]} which should make it easier to set the quotes correctly and is also easier to read/understand IMO. Up to you ;)

@claustromaniac
Copy link
Contributor Author

claustromaniac commented Apr 22, 2018

$bakfile in fClean $bakfile and the $1 in done < $1 > prefs.js should probably be wrapped in quotes, just in case

Yes, in double quotes, so $1 expands. 👍

can you refresh my memory what the difference is between done < and done <<< ?

< is redirection like in the CMD interpreter. <<< is a here string. There's plenty of documentation on that but, the short version as I undestand it is, the former redirects stdin to a file whereas the latter redirects to a string (the output of grep in this case).

not sure but I think the way you look for string-in-string is kinda risky. There are no separators between prefs, right? For example looking for a.b.c would match if there's a different pref a.b.c.d AFAICS, no?

after having added separators, shouldn't ${BASH_REMATCH[1]} also be wrapped in quotes, just in case?

As keen as always, I see. That was a total oversight on my part. Thanks :) I'll look into it.

Test if it does NOT contain a string: if [[ ! "abc" =~ "d" ]]

The problem I saw with that is the prefnames include special characters (most notably DOT), that would have to be escaped (I think). I initially considered using arrays for that, but I figured the overhead would be worse since I would have to loop over the whole array once for each line in prefs.js. Also, the code wouldn't look as clean that way IMO. IDK.

@earthlng
Copy link
Contributor

earthlng commented Apr 22, 2018

The problem I saw with that is the prefnames include special characters

good point, I didn't think about that

prefsCleaner.sh Outdated
## create backup folder if it doesn't exist
mkdir -p userjs_backups;
bakfile="userjs_backups/prefs.js.backup.$(date +"%Y-%m-%d_%H%M")"
mv prefs.js "${bakfile}" && echo -e "\nprefs.js backed up: $bakfile"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another bit borrowed from @overdodactyl's code. This got me thinking... should I add more code comments to this thing? I usually add them very sparingly (never really got used to it).

prefsCleaner.sh Outdated
while [ -e webappsstore.sqlite-shm ]
do
echo -e "\nThis Firefox profile seems to be in use. Close Firefox and try again.\n"
read -p "Press any key to continue."
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not fully convinced about this part. It's all or nothing. Thinking ahead, it might be better to do the check just once (not looping) in case webappsstore.sqlite-shm becomes a permanent file someday.

@earthlng
Copy link
Contributor

earthlng commented Apr 23, 2018

The script doesn't work in my linux vm. Haven't you tested it? I have to say I'm a bit disappointed :)

The problem is that the 1st while sees the full grep output as 1 line (= it only resets the 1st pref it finds).
I imagine the regex array has all prefnames because of it and you could either loop over that, or just redirect the grep output into a temp file and delete it again afterwards.

And does it really need the || [[ -n "$line" ]] in the 1st while loop? I don't think so.

This works for me and produces the same prefs.js as the Windows version when ran against the same prefs.js and user.js:

  grep -E "${prefexp}" user.js >/tmp/userjs_updater_temp.dat
  while read line; do
    ## blah blah
  done < /tmp/userjs_updater_temp.dat

@earthlng
Copy link
Contributor

webappsstore.sqlite-shm only exists if something happened to write to localStorage but the absence of it doesn't necessarily mean a running FF isn't using this profile. It's better than nothing I guess.

prefsCleaner.sh Outdated
mv prefs.js "${bakfile}"
if [ ! $? ]; then
fQuit 1 "Operation aborted.\nReason: Could not create backup file $bakfile"
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mv prefs.js "${bakfile}" || fQuit 1 "Operation aborted.\nReason: Could not create backup file $bakfile"

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I originally wrote it the other way around (if it doesn't fail do stuff). The if clause was a remnant of it. Didn't think it through.

@claustromaniac
Copy link
Contributor Author

@earthlng Did you even test the thing ONLY removing the -r from while read -r line in the first loop before accusing me of not testing my own code (and dismissing @overdodactyl's test in the process)?

As I said, I don't mind writing the temp file but I really wish I could understand your vm lol

@earthlng
Copy link
Contributor

Yeah sorry about that. The VM is a very old and never-updated snapshot of an already running archlinux. 🤦‍♂️

But yes I tested all kinds of things and nothing except what I wrote above worked.

@claustromaniac
Copy link
Contributor Author

Alright, I trust you. I'll just archive this X file and move on then.

@claustromaniac
Copy link
Contributor Author

@overdodactyl I'm using the same subfolder as the updater for the backups (even though the name is userjs_backups). What do you think? Would you rather have them in separate folders?

@earthlng
Copy link
Contributor

Woohoo! wrapping it in double-quotes works!

done <<< "`grep -E "$prefexp" user.js`"

works with and without -r, too. IDK, is the -r necessary?

@earthlng
Copy link
Contributor

Damn I'm sooo sorry man, now you already wasted even more time making changes for me! SOOOORRY

@claustromaniac
Copy link
Contributor Author

claustromaniac commented Apr 23, 2018

Nah, don't be sorry. I'd say it's great that you had that issue with the script and not random users, since you could find a solution. Thanks 👍

is the -r necessary?

Most of the time, probably not. Unless there is some weird way it can break the logic (not aware of any), it's better to leave it there. It's just to interpret backlashes literally.

@earthlng
Copy link
Contributor

I prefer else over continue and think it's stupid to backup prefs.js files in userjs_backups, sorry :)

If you're happy with that, I'm ready to commit it.

@claustromaniac
Copy link
Contributor Author

claustromaniac commented Apr 23, 2018

it's stupid to backup prefs.js files in userjs_backups, sorry :)

That's why I asked @overdodactyl about it (read above). I don't really care about the name of the folder but if I were to have them spawn in a subfolder, I'd rather have them in the same subfolder as the user.js backups created by the updater. (I didn't pick the name of the folder).

It doesn't matter anyway. That can be changed anytime. Feel free to merge this if you want.

@earthlng earthlng merged commit b4f1b4d into arkenfox:master Apr 24, 2018
@earthlng
Copy link
Contributor

Thanks @claustromaniac !

@claustromaniac
Copy link
Contributor Author

Glad to contribute. Thanks for all the input, as always.

@claustromaniac claustromaniac deleted the prefsCleaner.sh branch April 24, 2018 16:26
@earthlng
Copy link
Contributor

np, thanks for your patience

@1xPdd
Copy link

1xPdd commented Jun 21, 2018

Hi @claustromaniac you caught me while real life was giving me a beating. But since I asked for this, I wanted to thank you. I just now ran it on my main profile and, as far as I can tell, it worked great. So thanks. Cheers.

@claustromaniac
Copy link
Contributor Author

@1xPdd,

I hope real life is treating you better now, and I'm glad the script works well for you. Be sure to let me know if you ever have any issues with it or if you think it can be improved. I'll try to make some of the changes discussed here as soon as I manage to escape my current procrastination cycle, so don't be shy.

@atomGit
Copy link

atomGit commented Feb 3, 2020

hello @claustromaniac :)

fFF_check() {
	# there are many ways to see if firefox is running or not, some more reliable than others
	# this isn't elegant and might not be future-proof but should at least be compatible with any environment
	while [ -e webappsstore.sqlite-shm ]; do
		echo -e "\nThis Firefox profile seems to be in use. Close Firefox and try again.\n"
		read -p "Press any key to continue."
	done
}

i don't think this is working - i never see this message if i run the script while FF is running

would something like this be better perhaps? ...
Bash script to check running process - Stack Overflow

@rusty-snake
Copy link
Contributor

Confirming:

$ ls webappsstore.sqlite*
webappsstore.sqlite  webappsstore.sqlite-wal

Alternative I would suggest to use lock instead of webappsstore.sqlite-shm.

@atomGit
Copy link

atomGit commented Feb 3, 2020

in that case something like this maybe ???...

iFile=`ls "$sFirefoxDir" | grep '^lock$' | wc -l`

@earthlng
Copy link
Contributor

earthlng commented Feb 10, 2020

Does lock get reliably deleted on close?
Maybe favicons.sqlite-shm would be a better candidate? Do you guys have that?

@rusty-snake
Copy link
Contributor

Does lock get reliably deleted on close?

Yes

Maybe favicons.sqlite-shm would be a better candidate? Do you guys have that?

No

@atomGit
Copy link

atomGit commented Feb 10, 2020

i don't have any *shm files in the profile folder on linux

@earthlng
Copy link
Contributor

@rusty-snake are you on linux too, or Mac?

@rusty-snake
Copy link
Contributor

Linux

earthlng added a commit that referenced this pull request Feb 10, 2020
look for `lock` file instead of `webappsstore.sqlite-shm` to detect if firefox is running or not (with this profile)

see #405 (comment) and follow-up comments.

Thanks @atomGit for reporting the issue and @rusty-snake for confirming it.
@earthlng
Copy link
Contributor

okay, done. Thanks guys

PatrickMcKenzier pushed a commit to PatrickMcKenzier/user.js that referenced this pull request Oct 10, 2022
look for `lock` file instead of `webappsstore.sqlite-shm` to detect if firefox is running or not (with this profile)

see arkenfox/user.js#405 (comment) and follow-up comments.

Thanks @atomGit for reporting the issue and @rusty-snake for confirming it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants