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]: Firefox "Clear browsing history and cache" also removes all bookmarks #273

Closed
ltguillaume opened this issue Oct 15, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@ltguillaume
Copy link

ltguillaume commented Oct 15, 2023

Description

"Clear Firefox history" ->"Clear browsing history and cache" simply removes all but a few SQLite database files from the profile, among which is places.sqlite, which does indeed contain the profile history (e.g. visited pages and input history).

It also contains, however, all the user's bookmarks (table moz_bookmarks)!

As such, the option currently removes all bookmarks without notifying the user of this.

OS

Win10 x64, Win11 x64 (not very relevant).

Firefox 115 ESR.

Reproduction steps

  1. Create a bookmark in Firefox
  2. Run privacy.sexy with "Clear Firefox history" -> "Clear browsing history and cache" enabled
  3. Run Firefox again, see that bookmark is lost, only the default bookmarks are present.

Scripts

:: ----------------------------------------------------------
:: -------------Clear browsing history and cache-------------
:: ----------------------------------------------------------
echo --- Clear browsing history and cache
set ignoreFiles="content-prefs.sqlite" "permissions.sqlite" "favicons.sqlite"
for %%d in ("%APPDATA%\Mozilla\Firefox\Profiles\"
            "%USERPROFILE%\Local Settings\Application Data\Mozilla\Firefox\Profiles\"
        ) do (
    IF EXIST %%d (
        FOR /d %%p IN (%%d*) DO (
            for /f "delims=" %%f in ('dir /b /s "%%p\*.sqlite" 2^>nul') do (
                set "continue="
                for %%i in (%ignoreFiles%) do (
                    if %%i == "%%~nxf" (
                        set continue=1
                    )
                )
                if not defined continue (
                    del /q /s /f %%f
                )
            )
        )
    )
)
:: ----------------------------------------------------------

Solution

The only way to properly handle this is by doing some SQL queries to only clear the tables that save history entries.

@ltguillaume ltguillaume added the bug Something isn't working label Oct 15, 2023
@undergroundwires
Copy link
Owner

This is too bad. I had this kind of data loss before and it's really frustrating. I don't understand how this script could survive for years without this side-effect being detected.. Thank you for the report.

I plan to do like this:

  • Do a hot-patch on privacy.sexy web version. Remove this script until real fix is there.
  • Work on real fix, align the behavior to be same as Linux configuration, i.e. do not recommend the script at all, document properly and remove files selectively instead of iterating every file.
  • Then later in future, in next minor version i.e. 0.13.0, I will do an improvement to how collection files are loaded. I will allow scripts to be shared throughout all three OSes. This way, Firefox configurations and similar can be same on all operating systems. I want to start working on 0.13.0 but the fixes get the most priority so I haven't been able to reach the point of adding new features for months..

@ltguillaume
Copy link
Author

Thanks, I didn't know the documentation was already mentioning this for the Linux variant. But it doesn't seem like it handles it any different, though.

If you really want to provide this properly, it would need SQL queries (on Windows e.g. with https://sqldocs.org/sqlite/sqlite-with-powershell/).

Otherwise, my personal belief is that it would be better to not provide this option at all, instead of having it "half-assed" and potentially user data destroying. Even if it is documented, like for the Linux implementation. There are lots of other tools that do handle this properly, so best to use one of those then.

@undergroundwires
Copy link
Owner

I've played around with the implementation on Linux, and it's working alright (see CleanTableFromFirefoxProfileDatabase). The code is orphaned, not used, and it deletes the entire table. Better queries would be needed for some operations.

macOS and Linux (most major desktop distributions) come with SQLite and python by default, making life easier. For Windows, as you pointed out, a separate SQLite installation is needed. This discussion PowerShell/PowerShell#6050 provides another option too. So we could install it/run the query/uninstall.

So we can edit places.sqlite without touching bookmarks. But I do not have time to implement this for now. So as you say, two options: get rid of the script completely, or keep it but in isolation. I use this kind of cleanup when I will hand-over a computer to someone else or after someone else uses my computer. So there's a use-case for it: easy quick cleanup. I think there's value in giving a quick way to get rid of the data for now.

But on the other side this script should not cause any unexpected data loss without expectation.. So I guess not including it any recommendation pool, and document it properly on title that it deletes bookmarks would be fine. I also plan to add red light icon to unrecommended scripts, green to recommended, yellow to strict in 0.13.0 to highlight that.

I don't dictate but provide my opinions so I'm still open to any ideas.

@ltguillaume
Copy link
Author

Yeah it's pretty unfortunate that on Windows there's no native support for sqlite in this case. Other than that, I don't think there's a real solution, though.

undergroundwires added a commit that referenced this issue Nov 2, 2023
This commit unifies some of the logic, documentation and naming for
Firefox clean-up with improvements on both Linux and Windows platforms.

Windows:

- 'Clear browsing history and cache':
  - Not recommend.
  - Align script name and logic with Linux implementation.
  - New documentation and not including the script in recommendation
    provides safety against unintended data loss as discussed in #273.
- 'Clear Firefox user profiles, settings, and data':
  - Rename to 'Clear all Firefox user information and preferences' for
    improved clarity.
  - Add more documentation.

Linux:

- Replace `DeleteFromFirefoxProfiles` with
  `DeleteFilesFromFirefoxProfiles`.
- Migrate implementation to Python:
  - Add more user-friendly outputs.
  - Exclude removing directory itself for additional safety.

Both Linux and Windows:

- Improve documentation for:
  - 'Clear Firefox user profiles, settings, and data'
  - 'Clear Firefox history'
@undergroundwires
Copy link
Owner

Fixed and released in 0.12.6 🚀 I know that this is not the ideal solution we want, but it's better documented/safer implemented and not recommended anymore.

@ltguillaume
Copy link
Author

Good improvement nonetheless. Thanks!

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

2 participants