Acquired Opera AdBlock code

files/lists.js

+const lists = {
+        apply: function(key, add) {
+                //opera.postError(key + ' will be ' + (add ? 'added' : 'removed'));
+                var content = preferences.array(key + '-content');
+                switch(sources.style[key]) {
+                        case 'adblock':
+                                for(pos in content)
+                                        parse.adblock(content[pos], add);
+                                break;
+                        case 'opera':
+                                for(pos in content) {
+                                        if(add)

[Felix Dahlke, 2012/10/05 15:47:16]

Shouldn't this use parse.opera()? It has the very same logic. It's commented out
right now though.

+                                                opera.extension.urlfilter.block.add(content[pos]);
+                                        else
+                                                opera.extension.urlfilter.block.remove(content[pos]);
+                                }
+                                break;
+                }
+                //opera.postError(key + ' was ' + (add ? 'added' : 'removed') + ' with success (' + pos + ' elements)');
+        },
+        setup: function(key) {
+                for(key in sources.all()) {
+                        if(this.status(key))
+                                this.apply(key, true);
+                }
+        },
+        whitelist: {
+                get: function() {
+                        return preferences.array('whitelist-content');
+                },
+                set: function(array) {
+                        preferences.array('whitelist-content', array);
+                },
+                apply: function() {
+                        var content = this.get();
+                        for(i in content)
+                                opera.extension.urlfilter.allow.add(content[i]);

[Felix Dahlke, 2012/10/05 15:47:16]

See comment in parse.js on urlfilter.allow.

+                },
+                add: function(element, useMessage) {
+                        var content = this.get();
+                        if(content.indexOf(element) == -1) {
+                                content.push(element);
+                                this.set(content);
+                                if(useMessage)

[Felix Dahlke, 2012/10/05 15:47:16]

From what I've seen, useMessage is always true. Why parameterise?

+                                        opera.extension.postMessage({request: 'whitelist', type: 'add', rule: element});
+                                else
+                                        opera.extension.urlfilter.allow.add(rule);
+                        }
+                },
+                remove: function(element, useMessage) {
+                        var content = this.get();
+                        var position = content.indexOf(element);
+                        if(position != -1) {
+                                content.splice(position, 1);
+                                this.set(content);
+                                if(useMessage)

[Felix Dahlke, 2012/10/05 15:47:16]

As in add, seems to always be true.

+                                        opera.extension.postMessage({request: 'whitelist', type: 'remove', rule: element});
+                                else
+                                        opera.extension.urlfilter.allow.remove(rule);
+                        }
+                }/*,
+                make: function(url) {
+                        return url.replace(/[a-z]+:\/\//, '||').replace(/\/.*$/, "^*");
+                }*/
+        },
+        refresh: function() {
+                var success = true;
+                for(key in sources.all()) {
+                        if(preferences.bool(key)){ //If the list is enabled, update it
+                                if(!this.update(key)) //In case of error with the update
+                                        success = false;

[Felix Dahlke, 2012/10/05 15:47:16]

We can just return false here, avoiding the success variable and unnecessary
iterations.

+                        }
+                        else //If the list is disabled, remove it
+                                this.disable(key);
+                }
+                return success;
+        },
+        time: function(key) {
+                return preferences.int(key + '-time');
+        },
+        update: function(key) {
+                var value;
+                if(sources.style[key] == 'opera')
+                        value = download(sources.url(key)).replace(/(^|\n);.*?(\n|$)/gm, '$1').split('[exclude]')[1].split('\n'); //Pre-remove comments to save space

[Felix Dahlke, 2012/10/05 15:47:16]

This line definitely deserves its own function, with multiple lines.

It won't be able to handle ill-formatted filters like this. Or download
problems. And why is everything before the exclude section ignored?

+                else  if(sources.style[key] == 'adblock')
+                        value = download(sources.url(key)).replace(/^\[adblock.*\n/im, '').replace(/(^|\n)(!|##|#@#).*?(\n|$)/gm, '$1').split('\n'); //Pre-remove comments and unsupported to save space

[Felix Dahlke, 2012/10/05 15:47:16]

As above, this deserves its own function and multiple lines. I haven't checked
the whole code base at this point, but it seems like element hiding isn't
supported (apart from including e.g. fanboy's pre generated CSS).

+                //opera.postError(value.join("\n"));
+                if(value instanceof Array) {
+                        preferences.array(key + '-content', value);
+                        preferences.string(key + '-time', getTime());
+                        this.apply(key, true);
+                        return true;
+                }
+                else

[Felix Dahlke, 2012/10/05 15:47:16]

else is not necessary here. In fact, the return value doesn't seem to be used
anywhere.

+                        return false;
+        },
+        status: function(key) {

[Felix Dahlke, 2012/10/05 15:47:16]

How about "enabled" instead of "status"?  Status implies it's more than a
boolean IMO.

+                return preferences.bool(key);
+        },
+        disable: function(key) { //Disables a source
+                this.apply(key, false);
+                preferences.remove(key + '-time');
+                preferences.remove(key + '-content');
+                preferences.bool(key, false);
+                return true;

[Felix Dahlke, 2012/10/05 15:47:16]

The function will always return true no matter what. Probably not necessary.

+        },
+        enable: function(key) { //Enables a source
+                this.update(key);
+                preferences.bool(key, true);
+        }
+};