crnkv
commented
7 months ago Oops, I was missing out one ingredient: this bug takes effect if and only if setting.pageAction == true
I was testing the addon with this setting, so I didn't notice that when this setting was turned off, function ta_bg.setupCacheList() would stop and return at line 175, before the error in line 178 could happen. As such, ta_bg.setupAutoClear() could run without error.
Nevertheless, users who have setting.pageAction == true still encounter this bug, and when the bug is fixed in the future, their accumulated cache could still cause lag with the 1-minute-alarm mechanism (and also the unexpected cache day lost issue I mentioned before), which is the reason why I suggest use the browser.idle API instead.
BTW, I don't know whether the whole pageAction feature is broken or not, but if @wildskyf doesn't want to maintain this feature, it's better to remove all pageAction related codes, otherwise it causes AutoClear malfunction as well.
Required:
This is related to issue #117 : "Show Icon at navigation bar" setting does not work. I noticed that after I set up an AutoClear time limit, nothing happened, textarea cache of ~2 years (since I installed the addon) remained untouched. I used the firefox about:debugging Developer Tool to step into the scripts and found:
This bug caused the disappearance of the navigation bar icon and broke tha AutoClear functionality. I don't know since when this bug existed, but it seems to be related to the commit 1718656 which deleted the "page_action" line from manifest.json Since the "browser_action" line is still in manifest.json, so the browserAction API permission is granted as expected, but the pageAction API permission is absent. I will open a bugfix pull request very soon.
But once the pageAction bug is fixed, the AutoClear functionality back online, on its first execution, I'm afraid that another performance issue might appear. People like me who has never set up an AutoClear time limit would have accumulated tons of cache. checkAutoClear() will go through all of them and perform the deletion one by one. Due to performance issues, this could possibly take >1 min, then the next 'check-auto-clear' alarm would fire up checkAutoClear() again, overlapping with the previous ongoing one and causing problems. In principle I should open a separate issue, but this issue hasn't happened yet, so I'll simply propose a solution here and open a separate pull request implementing it later. I suggest use the browser.idle API to check outdated cache when the user hasn't generate any input for 3 min, like the way how the Expire History By Days addon has done. Reasons: Firstly, during checkAutoClear()'s first run, it will have enough time to clean up all outdated cache, while the next run will be when user come back to the computer, do some activity, and leave the computer again for 3 min. This should avoid overlaps. Secondly, people like me may prefer to keep a relatively heavy cache (from months to 1 year), so each checkAutoClear() run might cause perceptible lags in the worst-case scenario. Instead of doing it once a minute, it's better to do it when the user is away from keyboard. Thirdly, users may not be aware that any change in options takes effect immediately, so when AutoClear is back, a user might unintentionally change autoClear_day from 210 to 10 before he types 310. If the old 'check-auto-clear' alarm happens to fire up when autoClear_day value is 10, then the user would unexpectedly lose 200 days of cache. Using the idle API avoids this.