[Ext] Adblock Plus 0.5.11.4

[lymanepp]

I started using Adblock Plus in the last several days. I really, really like the automatic synchronization feature for managing my five computers. I have noticed several times that the automatic synchronization has downloaded a partial file. My adblock file on the server is 4,471 bytes long. If I export the partially downloaded adblock filters, the file is exactly 4,096 bytes long.

I believe I have found the source of the problem in adblockLoadSettings() in component.js.

if (autoSync == true) {
/* ... */
input = streamIO.read(stream.available());
/* ... */
}

Including HTTP headers, it would require four TCP packets to receive my adblock file. It is normal for TCP packets to arrive at different times. According to the documentation, nsIInputStream::available() function returns the "number of bytes currently available in the stream". Problems occur when only one, two or three of the TCP packets has been received when calling nsIInputStream::available(). To fix this we need to know how many bytes to expect. The nsIChannel ::contentLength attribute contains the expected length.

Here is a version that works for me.

if (autoSync == true) {
/* ... */
var length;
if (channel.contentLength > 0)
length = channel.contentLength;
else
length = stream.available();
input = streamIO.read(length);
/* ... */
}

Edit: Changed to add more information and finally a solution. Yeah!

[mdew]

I've also noticed this partial file error when using synchronization feature.

[mdew]

ok, i've noticed some nasty bugs with that synchronization code lymanepp, heres my <a href="http://fanboy.co.nz/adblock/AdBlock_Plus_0.5.6+_icon.xpi">.xpi</a> I hacked using your code, basically if I enable the sync It'll block everything, remove the automatic sync, and the list works as normal. Your'e free to test it out on my list.

[mcm_ham]

Thanks lymanepp for discovering the issue and finding a solution. It's not an error I've experienced, I was synchronizing from the local disk instead of from a website. However, I've included your patch into the extension. Can you and mdew make sure that it is working correctly now. mdew, in the last line he forget to minus one from the length such that an extra character was read which was interpreted as a blank filter that blocked everything.

Also I gather there is a problem with the new ability to collapse DIV sections under Mozilla 1.7.7 not always working correctly. I'd be interested if people could test it and confirm the issue with Firefox as well. See this site for information about blocking DIV sections.

[lymanepp]

in the last line he forget to minus one from the length such that an extra character was read which was interpreted as a blank filter that blocked everything.

With minus one on the length, I lose the last byte of the file (downloaded with http). Mine doesn't indiscriminately block web sites without the minus one on the length. I'm using Firefox 1.0.4--is anyone with the lock-up problem using a different version? Should the minus one only apply to the stream.available() results?

Edit #1: I figured out what's happening. I am using Filterset.G saved to a common location. Filterset.G is not terminated with a '\n' but mdew's adblock file is. It would be good if adblock could handle both formats.

Edit #2: This is handled by fillList() in settings.js when importing a file. There's a comment in fillList() that "only non-empty patterns" get added to the list. We need something like that when synchronizing.

Edit #3: I am still getting sporadic partial downloads. Even when the content length is set correctly. I wonder if a read loop is needed to read until "length" bytes have been read. That is typical for socket applications.

[ahpatel]

I wrote a script that allows me to check filtersetG for updates and download any new updates to a statically named file -- allowing me to automaticaly sync with the filtersetG patterns. However, Adblock 0.5.6+ doesn't allow me to use local files for synchronization. As a workaround, I've set the script to upload the filters to my own private webserver, but <b>I was wondering if the code could be easily modified to allow syncing with local files (i.e., C:\filterfile.txt)?</b>

[lymanepp]

Below is an excerpt of the code that fixes all outstanding problems (it removes empty lines and handles partial downloads). It has been tested with Filterset.G and mdew's filters. I haven't been able to recreate the partial downloads with these changes.

<code>
var input = "", inputArray, linebreak, inputLine = "", length;

streamIO.init(stream);
if (channel.contentLength > 0) length = channel.contentLength;
else length = stream.available();
while (input.length < length)
input += streamIO.read(length - input.length);
streamIO.close();
stream.close();
linebreak = input.match(/(?:\[Ad[Bb]lock\])(((\n+)|(\r+))+)/m)[1]; // first: whole match -- second: backref-1 -- etc..
inputArray = input.split(linebreak);

var headerRe = /\[Ad[Bb]lock\]/; // tests if the first line is adblock's header
if (headerRe.test(inputArray[0])) {
inputArray.shift();

inputLine = inputLine.concat(inputArray[0]);
for (var i = 1; i < inputArray.length ; i++) {
if (inputArray[i].length > 0) // only non-empty patterns!
inputLine = inputLine.concat(" ",inputArray[i]);
}

Branch.setCharPref('patterns', inputLine);
list = inputLine;
}
</code>

Edit: fixed length in call to streamIO.read()

[mcm_ham]

Well that works for me so I've added it, thanks lymanepp, I've been unstuck by the issue you mentioned in your first edit before.

illegalien, you don't need to modify the code to get it to work with filter list files on the locale disk. Just open the text file in Firefox and copy the URL to the add synchronization path box. On a network I get something like this:

Code: Select all

file://///bedroom/SYSTEM%20(C)/documents/text.txt

[lymanepp]

Well that works for me so I've added it, thanks lymanepp, I've been unstuck by the issue you mentioned in your first edit before.

Thanks for integrating my changes. I noticed one potential issue in the change to settings.js. I'm not sure if it's necessary, but input doesn't get initialized to "" in the synchronize() function.

It would probably be a good idea to combine the download code from synchronize() in settings.js with the download code from adblockLoadSettings() in component.js. It will be easier to keep them working the same if they are the same code. Just my $0.02.

[mcm_ham]

At the moment when you manually synchronize it checks to see if it's null and if so will load the filters that were bundled with the extension. Where it automatically synchronizes it's in a try-catch block that handles the case if it was null there, meaning no synchronization will occur.

Indeed combining the code somewhere is a good idea and something I was planning. I had left it seperate when I had wrote it at the time though because there are a few differences like whether it overwrites or not but that could be easily handled by having another input to the shared function.

[Zulithe]

mcm_ham, love your changes. Can you do anything about that how Adblock changes the icon shown when an image is still loading/gone to that awful looking red diamond?

[AnonEmoose]

Regarding blocking DIV's. I posted this idea in the AdBlock forum. http://aasted.org/adblock/viewtopic.php?p=10938#10938 . I am crossposting here, as these forums are more widely read

Adblock is popular so that some sites don't use images or flash advertisings but insert the ads directly in the HTML page. But for layout reasons, the ads are encapsulated in DIV sections with 'class' attribute, which makes it possible to identify - and BLOCK - them

Might I make a suggestion?? Many sites use an "id" for many of their DIV's. A small change could enable finer grain control to utilize the div.id instead div.className as id is more precise than className

Code: Select all

if ( div.id !="" ) {

Code: Select all

 fakeurl.spec = doc.URL + "#DIV(" + div.id + ")";

and use div.className when div.id is null/empty. I did a quick dirty hack and it seems to work well (especially on Yahoo which utilizes id on many of their divs.)

Note: if you think is ok, perhaps fakeurl.spec should be differentiated

Code: Select all

fakeurl.spec = doc.URL + "#DIV(id:" + div.id + ")";

Code: Select all

 fakeurl.spec = doc.URL + "#DIV(class:" + div.className + ")";

EDIT:
This is the code for the quick hack in AdBlock's components.js
<code>var div = divs[i].QueryInterface( Components.interfaces.nsIDOMHTMLDivElement );
+ if (div.id !="") { var divAttribute = div.id; var nameAttribute = "id"; } else { divAttribute = div.className; nameAttribute = "class"; }
+ if ( divAttribute !="" ) {
- if ( div.className!="" ) {
var doc = null, pn = div.parentNode;
do {
try {
doc = pn.QueryInterface( Components.interfaces.nsIDOMHTMLDocument );
} catch(e) {}
pn = pn.parentNode;
} while ( doc==null && typeof(pn)!='undefined' );
if ( typeof(doc)!='undefined' ) {
var filtermatch = null, fakeurl = new Object();
- fakeurl.spec = doc.URL + "#DIV(" + div.className + ")";
+ fakeurl.spec = doc.URL + "#DIV("+ nameAttribute + ":" + divAttribute + ")";
fakeurl.host = "all";
</code>

[Morac]

I'm running Firefox 1.0.4 for Windows XP SP2 and I upgraded from Adblock 0.5.6 to 0.5.6+ (May 25) and now the Extensions Manager window is blank (no extensions, but the bottom 3 buttons are still there) when I open it. Not only that but if I close the blank Extensions Manager window and try to open it again, nothing happens and when I go to shut down Firefox it hangs and I need to kill it using the task manager.

I tried disabling adblock, removing all the filters and disabling all the options (site blocking, etc) and still it did not work.

Even though the window opened as blank I could installed the 0.5.6 version on top of the May 25 version and when I closed and opened Firefox everything worked again. I then started trying the 0.5.6+ versions one at a time to see when they stopped working. The May 0.5.6+ May 22 version was the first build where I experienced this problem so it was something that was added to that build.
To see if it was the DIV change, I also tried Rüschi's version which worked fine so that isn't the problem..

It does work in a new profile, which means that most likely adblock is interacting with another extension. The question though is why is AdBlock even acting on the Extension Manager window to begin with?

[mcm_ham]

Zulithe, I'll have a play around with the icon version, I use the text version and it doesn't seem to change to unloaded which is when the red icon would show. AnonEmoose, thanks for your enhancement suggestion I'll let Rueschi look at it since it was his original hack, but I see no reason not to include it.

Morac, I know the cause of the issue so thank you for bringing it to my attention. It is a result of this fix:

-Fixed error in javascript console that's caused after opening extensions manager (from official).

Basically Adblock used an extension manager overlay to display it's information in the manager window. On the trunk builds of Firefox this no longer worked and was throwing an error. Since it didn't actually add any new information compared to what the install.rdf already provides I removed it. It seems though that when you upgrade from an older version of Adblock Plus or from the original the extension manager overlay is still registered. Except when I opened it the second time it didn't freeze for me but actually worked.

What you would have to do is uninstall Adblock to remove the old chrome registry before installing the new version. I've put the xul overlay file back in so that error should no longer occur if the chrome registry entry is left behind from a previous install as a result of upgrading since it will now still be able to find the overlay file.

[Morac]

What you would have to do is uninstall Adblock to remove that chrome registry before installing the new version. I've put the xul overlay file back in so that error should no longer occur if the chrome registry entry is left behind from a previous install as a result of upgrading since it will still find the overlay file now.

Ahh, Firefox bug 278534. I didn't even consider that.

Thanks for figuring out the problem so quickly. You could leave the overlay out and just make sure people know they have to uninstall Ad block before installing the latest version, but putting the overlay back in works too.