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

[BUG]: Reverting Firefox settings do not work on Linux #282

Closed
undergroundwires opened this issue Nov 6, 2023 · 12 comments
Closed

[BUG]: Reverting Firefox settings do not work on Linux #282

undergroundwires opened this issue Nov 6, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@undergroundwires
Copy link
Owner

undergroundwires commented Nov 6, 2023

Description

Removing lines user.js does not cause Firefox to update prefs.js.

Unintended and confusing situation:

  1. User sets something via privacy.sexy
  2. user.js gets created/modified with the respective settings
  3. User opens Firefox
  4. Firefox copies over the settings from user.js into prefs.js
  5. User tries to revert said settings via privacy.sexy
  6. The line gets removed from user.js by privacy.sexy
  7. User starts Firefox thinking the setting is reverted, but because the setting is still present in prefs.js, it is in fact still in effect

Suggested solution

a) privacy.sexy should remove the line from prefs.js as well as user.js.
b) privacy.sexy remove the prefs.js completely to force recreation of it.

OS

Linux

Reproduction steps

  1. On Firefox, go to about:config, search for network.captive-portal-service.enabled, verify that the default value is true.
  2. Go to privacy.sexy, search for network.captive-portal-service.enabled, download and apply the script.
  3. Restart Firefox, go to about:config, search for network.captive-portal-service.enabled, verify that the value is false.
  4. Go to privacy.sexy, revert script with network.captive-portal-service.enabled, apply the settings, this will delete the preference from user.js.
  5. Restart Firefox, go to about:config, search for network.captive-portal-service.enabled, the value is still false.

Additional information

Reprted by @ltguillaume in #232 (comment), created this issue for easier management.

@atomGit
Copy link

atomGit commented Nov 7, 2023

i don't know the best way to handle this - possibly diff the matching prefs and let the user decide what to do

off the top i'm not seeing a clean way to revert changes in prefs.js because who knows what the user has done or wants to keep since applying the sexy scripts

i'm not sure user.js should be deleted on reverting the changes either unless they're prompted prior

maybe a better way to handle all this would be to create user.js.txt and let them do whatever they want with it which alleviates the need to revert anything

@atomGit
Copy link

atomGit commented Nov 7, 2023

ps: i'll be happy to write a guide for user.js.txt if you go that route

@undergroundwires
Copy link
Owner Author

Thank you for the input @atomGit .

If I get this right, you suggest that we create a user.js.txt file and then ask user to modify their user.js accordingly?

I have two concerns about this:

  1. It does not address the prefs.js issue. Even though if they change their user.js, the same problem will still be there.
  2. Why would we not change the settings directly if the user asks for it using the UI?

Point 2 is also about prompting. Shouldn't we consider the whole UI of privacy.sexy is a big prompt screen where people explicitly consent to do changes? Showing other types of prompts would be tricky. There are users, or even derivative tools that are relying on automated scripts generated by the tool. It would break many use-cases if we start prompting for changes. This is complex, requires planning, user expectation management, providing options etc.

Other options:

a) Remove the line from prefs.js as well as user.js as initially suggested.
b) Remove the prefs.js completely to force recreation of it. I tested this and on every start of Firefox it re-creates pref.js with latest defaults.

I'm not sure which one is the "safer" option but a) sounds the least intrusive and probably the safest. Mozilla devs discuss about dropping user.js competely, the idea seems to be dropped, as the issue is years old with no recent activity. However, it suggests to me more that option b) is not the good one, neither relying heavily on modifications to user.js as sustainable long-term strategy.

Is there any chances that we mess up things by removing lines from prefs.js? Is it worth creating a backup like prefs.js.backup (if not exists) with current prefs.js before applying, or do we just bloat user profile data?

@atomGit
Copy link

atomGit commented Nov 12, 2023

you definitely don't want to delete prefs.js - that could cause a lot of trouble

if you want to accept responsibility for reverting changes, then it might be best to read the current value of each pref (prefs.js) that you want to change and store them as commented lines in a "DO NOT TOUCH" section of user.js - any new prefs that don't yet exist can be ignored

so on restore, you use the values of the commented/backup prefs to change the values in prefs.js and just delete any prefs that didn't exist in the first place, then either delete user.js or, as you suggest, delete the lines (plus the backup section)

this it still problematic, but perhaps acceptable ... problematic because, imagine a default value of 1 for a pref, then your script flips it to 2, then the user flips it to 3 at some point, then decides to run the script to un-do the changes, so now when you restore you're overriding their choice

one way to address this might be to re-read the current values and if a value matches neither what the script set it to, or the backup value, then ask the user what to do

the safest option would be to add/change whatever prefs you want and let it up to the user to restore - that's why i suggested user.js.txt with a comment section explaining things - but i think this could all be handled reasonably well as long as you're careful about how you restore

@atomGit
Copy link

atomGit commented Nov 12, 2023

just to be clear about what i mean by potentially causing a lot of trouble by deleting prefs.js and restoring the defaults ... any UI changes they make are gone (toolbars, etc.), and any changes they make in settings are gone, and any prefs added by extensions are gone, and there's potentially still more damage that can be done, including data loss

@ltguillaume
Copy link

Two things:

  • This issue is present on all OS's, not just Linux
  • Indeed, deleting prefs.js is absolutely not an option

@undergroundwires
Copy link
Owner Author

undergroundwires commented Nov 13, 2023

I have two points (feel free to question or challenge, I'm not strong about these, but need input). The goal is to provide as much as user friendliness, easy of use meanwhile "doing the job" of fixing privacy issues.

So this is what I suggest:

image

We should default to software defaults instead of previous settings

Ok, deleting prefs.js is out of options. Now we talk about deleting the lines.

About your concern about restoring users new setting @atomGit, the current logic now skips if the value is different than what privacy.sexy sets, and does not revert it at all. Some people use revert options as "recovery" for configurations if they or other tools/scripts messes up those, this would not help in that case. So if a user has made multiple changes over time, including some not via privacy.sexy, the tool wouldn't provide a way to go back to the original settings from before privacy.sexy was ever used. So it's a trade-off, but I believe it's more user-friendly to not change options if they're not the ones privacy.sexy is setting. It respects the user's custom settings that may have been changed outside of privacy.sexy.

It' sounds like reasonable approach to store revert state data in user.js as comment. But what happens if Firefox changes its defaults? This happens often in new releases as we discussed, then we're not actually reverting to default Firefox state but to something else. This sounds less-safe than deleting the line. So we need to make an assumption, do we assume that:

  1. The user does wants to go back to the defaults before, this can be Firefox's new default that may change between updates.
  2. The user wants go back to the previous value before modification.

On both cases, the reverted state might not be "original" state the user expects. But I think reverting to Firefox defaults (option 1) is more consistent with existing behavior. Because, all of existing revert scripts are currently designed to restore the default software or operating system state, not to set the opposite values or previous values. To mitigate a potential problem for option 2, we add data for future manual restores.

Creating data for future manual restores

I see your point now, then a separate text file makes sense. I think we could do the restore/revert automatically, but provide necessary data about previous state. I'd happy to see a structure format for user.js.txt and follow your suggestion if you could give more insights. I may be better to create a separate file rather than bloating user.js.

So I suggest "doing the job" but after your mentioned concerns, we can still provide restore data for power-users. "Doing the job" is key here, as there are many great projects (which you also suggested), which does much better job at providing an opinionated user.js (desired state), but the difference of this tool is to automatically execute the changes. If we require user interaction (do you want to go back to previous values or defaults), it would be confusing for not so advanced users, and user interactions breaks simplicity and automation.

@atomGit
Copy link

atomGit commented Nov 13, 2023

current logic now skips if the value is different than what privacy.sexy sets, and does not revert it

sounds good

It' sounds like reasonable approach to store revert state data in user.js as comment. But what happens if Firefox changes its defaults?

good point

again, given that you're messing with a very complex piece of software and risking overwriting user choices on restore, i would recommend prompting the user and asking what value they want to restore, if any

I may be better to create a separate file rather than bloating user.js

i wouldn't worry about that - personally i think it's more intuitive to have both the pref and the comment in the same file that FF reads, which was my intention with adding the .txt extension where sexy adds the prefs along with explanations and then passes everything off to the user thereafter

If we require user interaction ... it would be confusing for not so advanced users

maintaining a user.js requires some knowledge and regular evaluation of the settings as they are added/changed/remove by moz - i don't know of any way around that unless one uses LibreWolf without a user.js ... and this brings up another point...

what happens when sexy flips a pref, like FPI for example, then moz introduces ETP? those using the script wouldn't have any way of knowing and may be left with conflicting prefs of a false sense of privacy

in my most respectful opinion, this whole thing ought to handed off either to LibreWolf (less-tech users) or arkenfox instead of reinventing the wheel - both already have the capability to keep the user.js updated relative to the latest FF release and both cover a much wider gamut of privacy related prefs (currently ~180 active prefs in arkenfox + all the inactive ones)

@ltguillaume
Copy link

I think this is all needlessly complicated.

  1. privacy.sexy suggests a setting
  2. If user enables it, the script will set it in user.js (either adding or overwriting the setting). This makes sure Firefox will honor this setting.
  3. If user reverts it, the script will remove the setting from both user.js and prefs.js, so that the setting will revert to Firefox's current default.

Done.

@undergroundwires
Copy link
Owner Author

undergroundwires commented Nov 25, 2023

I think what you're suggesting is the most viable next step. So I will do following changes.

  1. Detect whether Firefox is running, log warning (stderr) to terminal if it is currently running and tell users to close it before running the revert script. This is because modifications to prefs.js will be overriden when it's closed and will not persist.
  2. On revert, repeat the same modification process of users.js that we have for prefs.js .

Like this:

image

@ltguillaume
Copy link

This looks great!

undergroundwires added a commit that referenced this issue Nov 26, 2023
Improve the revert process for Firefox settings by extending
modifications to also include `prefs.js`.

- Validate profile directories similarly to execution script.
- Check and warn if Firefox is running during revert to prevent
  `prefs.js` from being overriden.
- Clarify output messages for execution and revert scripts.
- Add flowchart diagram for visual documentation.
- Improve documentation for consistency and precision.
- Update `.gitignore` to account for temporary draw.io files.
@undergroundwires
Copy link
Owner Author

undergroundwires commented Nov 27, 2023

Thank you for the issue and guiding the solution @ltguillaume and @atomGit. The fix is released since 0.12.8 🚀.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants