GNOME Bugzilla – Bug 697329
AdBlock / Ad Blocker feature sometimes has an empty filters list and does not work at all
Last modified: 2015-09-17 23:38:50 UTC
Epiphany's ad blocker doesn't seem to block many (or any?) ads right now. I unchecked "Allow advertisements" in the preferences, then visited yahoo.com, boston.com, times.com and sfgate.com . I still saw ads on all these sites.
I've seen this before too. I haven't figured out why or how, but the adblocker either works consistently forever (and it's EasyList, so it's actually a top-class adblocker), or doesn't work at all.
*** Bug 723090 has been marked as a duplicate of this bug. ***
Interesting finding from Michael in bug #723090: > "Next time this happens, check ~/.config/epiphany/adblock/filters.list > I discovered that mine was empty, not sure why...." I concur: today, I tested with http://lapresse.ca and Epiphany 3.12. I noticed that the filters.list file was 0 bytes in size, so I nuked the ~/.config/epiphany/adblock and ~/.config/epiphany/extensions/adblock (actually, the whole extension folder, since extensions don't exist anymore), and ad blocking started working as the files were regenerated on app restart.
I can to confirm the same behavior (exactly) here. Thanks, now adblock works again.
Silly workaround I've found: remove/regenerate the filters.list so it's not 0 bytes, then make the whole ~/.config/epiphany/adblock folder as read-only.
Created attachment 284162 [details] [review] Don't accidentally delete adblock filters The uri-tester is created from the web extension. When created, it opens ~/.config/epiphany/adblock/filters.list for reading in uri_tester_load_filters(), then calls uri_tester_set_filters() with the read filters. uri_tester_set_filters() unconditionally calls uri_tester_save_filters(), so we immediately write back what we read to filters.list. But this is racy: if you are starting multiple web processes at the same time, such as when opening epiphany with multiple saved tabs, then one process may open the file for reading after another has opened it for writing (which clears the file) but before the filters have been written back to the file, so now one UriTester instance has an empty list of filters, and it will immediately write back that empty list. The original list is completely doomed because the only time we ever write to filters.list is immediately after the filters are read, since we do not support modifying filters. That's right, these writes are NEVER necessary, so let's just remove them completely so we can be completely sure the problem is gone. Now we have an ununsued uri_tester_save_filters() function, but I don't want to get rid of it quite yet as I do want to support at least a couple different types of filters in the future (for tracking protection). Also, there are already other unused functions here as well, so one more is no difference for now, but refactor is imminent.
Just in the last week I removed the ~/.config/epiphany/adblock more than 30 times. Thanks, and again - thanks!
I don't understand why the writes are never needed, if load the filters on construction, but they are never written. I guess we need a different code path for the case when we download the filters, and when we load them from the disk (to not save them again). And also make sure the writes are atomic yo avoid the race conditions.
The writes would be needed if Epiphany was able to change the list of filters, e.g. if Epiphany allowed whitelisting particular sites. I'm thinking we might replace our "tell web sites I do not want to be tracked" setting with "Allow tracking" or something similar, and use that to toggle a subscription to EasyPrivacy tracking protection. Keep in mind that this only applies to the list of subscriptions. Saving the actual regex expressions might be racy too, and I should look into that, but that's not causing problems in practice. Anyway the patch is not really "right" -- it leaves too much unused code. I'm thinking that maybe the UI process should manage the subscription list and actually downloading the filters to disk, and the web process should only ever read. I'm about halfway through changing this.
(In reply to comment #9) > The writes would be needed if Epiphany was able to change the list of filters, > e.g. if Epiphany allowed whitelisting particular sites. I'm thinking we might > replace our "tell web sites I do not want to be tracked" setting with "Allow > tracking" or something similar, and use that to toggle a subscription to > EasyPrivacy tracking protection. re: whitelisting, I've filed bug 711683 to whitelist our default search engine, DuckDuckGo.
Comment on attachment 284162 [details] [review] Don't accidentally delete adblock filters I'm going to push this band-aid since this bug is super annoying and, as mentioned in comment #9, there is no valid reason for epiphany to ever write to filters.list right now. But this file needs a big cleanup so the bug should not be closed yet. Attachment 284162 [details] pushed as fecf8df - Don't accidentally delete adblock filters
I'm still seeing the issue with 3.14.x; I cleared any "adblock" folders from ~/.config/epiphany, and I flipped twice the setting for ad blocking in the Preferences dialog (just to be sure), stopped all instances of web/webkit, and still adblock doesn't work on one of my computers (it seems to work on the other, last time I checked). Looks racy to me...
*** Bug 739833 has been marked as a duplicate of this bug. ***
(In reply to Jean-François Fortin Tam from comment #12) > I'm still seeing the issue with 3.14.x; I cleared any "adblock" folders from > ~/.config/epiphany, and I flipped twice the setting for ad blocking in the > Preferences dialog (just to be sure), stopped all instances of web/webkit, > and still adblock doesn't work on one of my computers (it seems to work on > the other, last time I checked). Looks racy to me... I think bug #735309 is the cause of this issue.
*** Bug 755186 has been marked as a duplicate of this bug. ***
Let's use bug #735309 for the remaining issue, since it has a good explanation of the only race I've managed to find so far.