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;
+                                if(opera.version() >= 12.1) {
+                                        for(pos in content)
+                                                parse.adblock(content[pos], add);
+                                        break;
+                                }
+                                else {
+                                        for(pos in content)
+                                                parse.adblockOld(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);
         }
 };