Rayquaza01 / HistoryCleaner

Firefox addon that deletes history older than a specified amount of days.
MIT License
81 stars 6 forks source link

Manifest v3 (Chrome) - Idle Trigger #30

Open Rayquaza01 opened 8 months ago

Rayquaza01 commented 8 months ago

With Manifest v3, all event listeners in the background script must be at the top level. In History Cleaner, the idle listener is not set at the top level. Because the idle length is user set, we need to fetch the idle length from storage before creating the event listener (which can't be done at the top level), and the listener is removed if the trigger mode is not idle.

Because of this, having a user set idle length seems to be impossible with Manifest v3. Removing the event listener when it's not in use also seems impossible.

There are a couple ways to handle this:

Option 1: Deprecate the idle length option on Manifest v3, but keep idle support in. Idle length will always be 60 seconds. Even if the trigger mode is not idle, the idle event listener will still trigger (but it won't do anything).

Option 2: Deprecate idle entirely on Manifest v3. Change the default trigger mode to the new timer mode. (Triggers at a set interval, Defaults to 24 hours).

Personally, I'm leaning towards option 2. Clearing history on idle doesn't make a lot of sense (the cutoff time is set to midnight, so unless the extension behavior is set to delete all history, history is only actually deleted once per day anyway). Clearing history only once per day was the intention of the startup trigger mode, but if you always leave your browser open then it will only trigger once. With the timer trigger mode, it will still trigger once every day even if you always leave your browser open.

What I want feedback on is:

  1. Should the idle trigger be deprecated on Manifest v3? Or just idle length?
  2. Should the timer trigger be made default on all versions?

(Note: I am planning on keeping the Firefox version on Manifest v2 for the foreseeable future. Neither idle length nor the idle trigger will not be deprecated on the Firefox version. However, I am considering making timer mode be the default for new installs on all versions.)

ejona86 commented 7 months ago

I'm a fan of the new timer trigger. I waited a bit after the update to comment to see how well it works. Every time it fires it does something, and it fires rarely. The idle timer was most frequently a noop. But I think that behavior is dependent on which delete behavior.

herrbrixter commented 3 months ago

I'm fine to default to timer mode and depreciate idle mode. You could explain it better that timer 1440 means it deletes once every 24 hours. Just so it's easier to understand.

Rayquaza01 commented 3 months ago

Just as an update to this:

The idle trigger was deprecated in Chrome in 1.6.0, and the Timer was made default. Idle trigger is still the default in Firefox, however I am planning to change that for 1.6.2

(For clarity, 1.6.1 is being skipped because I made a mistake while publishing the Firefox version of 1.6.0, so 1.6.1 and 1.6.0 are the same version)

Due to WebExtension Polyfill not currently supporting Manifest v3 (see https://github.com/mozilla/webextension-polyfill/issues/329), I split the code base to remove the dependency on WebExtension Polyfill in Chrome. The Chrome source is under cr-src/ and the Firefox source is under src/. I'm planning to undo this split and remove the runtime dependency on WebExtension Polyfill entirely. (There will still need to be some duplicated files (such as the webpack config, manifest file, and background script), however much of the code works in both Manifest v2 and v3)