downthemall / anticontainer

DownThemAll! AntiContainer (Extension to a Firefox, Seamonkey extension)
Mozilla Public License 2.0
93 stars 41 forks source link

Merged filter expression is incompatible with the JS version #61

Closed poke closed 12 years ago

poke commented 12 years ago

As already announced in #60, there some bug which is why I had to disable the filter generation/merging again in 58cb1171a6.

It is mere coincidence what pref()s are parsed last. It relies on the enumeration of the "directory", may it be an OS enumeration or ZipReader enumeration. My guess is that the ZipReader returns dtaac.js, filters.js while the OS returns it the other way round (by coincidence).

As it turns out, the enumeration actually seems consistent, but my observation was simply wrong. When I install a blank AntiContainer XPI, the default filter is actually empty. It’s just that (usually) the filter generation runs directly so it does not make a difference. When looking at about:config and resetting that value however, it is empty. So it seems that the generated and merged filter expression is never actually used, hence this issue never occuring before.

With my build process changes the preferences are now only set once, so nothing can overwrite another, so the filter expression definitely makes it into the running extension. And there the following issue happens:

As soon as you open dTa, either the download dialog, the manager or the preferences, it wants to load the filter definition and fails with the following error:

Error: invalid quantifier
Source File: resource://dta/support/filtermanager.jsm
Line: 97

I’ve already tried debugging a bit, and the error happens when the merged expression is converted to a RegExp object. Apparently some quantifier ({ }, *, + or ?) is misplaced or not escaped correctly, but I don’t know what exactly happens (or should happen) there.

As that problem blocks the plugin completely, I disabled the filter generation temporarily, which basically results in the same as it was before (as the actually written filter was never read).

nmaier commented 12 years ago

We should drop the filter generation during build entirely. The merger in service.js should take care of it.

poke commented 12 years ago

That’s fine with me as well. However sometimes, the filter won’t get generated for me. Of course, randomly disabling/enabling plugins will trigger it, but the initial generation often fails.

For example in the example setup I did earlier for testing this exact problem, the filter still isn’t generated, although I did open the preferences/manager quite a few times. So would there be some way to additionally trigger the generation to make sure it’s actually built at least once?

nmaier commented 12 years ago

The filters will only be generated once the FilterManager get initialized. Which, depending on what you do and what dTa version you're using, may happen very late in the game. Specifically, FilterManager will push the DTA:filterschanged notification, which AC services.js listens for and rebuilds the filter as required.

There is some issue with the mergeids pref currently preventing filter-recreation too often, though. I'll tackle this within this issue as well.