Issue 4499 - Consider $generichide exceptions when applying element hiding rules

lib/child/elemHide.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 76 matching lines @@
   {
     return Ci.nsIAboutModule.HIDE_FROM_ABOUTABOUT;
   },
 
   newChannel: function(uri, loadInfo)
   {
     let match = /\?hit(\d+)$/.exec(uri.path);
     if (match)
       return new HitRegistrationChannel(uri, loadInfo, match[1]);
 
-    match = /\?css(?:=(.*?))?$/.exec(uri.path);
+    match = /\?css(?:=(.*?))?(&specificonly)?$/.exec(uri.path);
     if (match)
     {
       return new StyleDataChannel(uri, loadInfo,
-            match[1] ? decodeURIComponent(match[1]) : null);
+            match[1] ? decodeURIComponent(match[1]) : null, !!match[2]);
     }
 
     throw Cr.NS_ERROR_FAILURE;
   },
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIFactory, Ci.nsIAboutModule])
 };
 AboutHandler.init();
 
 /**
@@ skipping 94 matching lines @@
   suspend: notImplemented,
   resume: notImplemented,
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIChannel, Ci.nsIRequest])
 };
 
 /**
  * Channel returning CSS data for the global as well as site-specific stylesheet.
  * @constructor
  */
-function StyleDataChannel(uri, loadInfo, domain)
+function StyleDataChannel(uri, loadInfo, domain, specificOnly)
 {
   BaseChannel.call(this, uri, loadInfo);
   this._domain = domain;
+  this._specificOnly = specificOnly;
 }
 StyleDataChannel.prototype = {
   __proto__: BaseChannel.prototype,
   contentType: "text/css",
   _domain: null,
 
   _getResponse: function()
   {
     function escapeChar(match)
     {
       return "\\" + match.charCodeAt(0).toString(16) + " ";
     }
 
     // Would be great to avoid sync messaging here but nsIStyleSheetService
     // insists on opening channels synchronously.
     let [selectors, keys] = (this._domain ?
-        port.emitSync("getSelectorsForDomain", this._domain) :
+        port.emitSync("getSelectorsForDomain", [this._domain, this._specificOnly]) :
         port.emitSync("getUnconditionalSelectors"));
 
     let cssPrefix = "{-moz-binding: url(about:abp-elemhide?hit";
     let cssSuffix = "#dummy) !important;}\n";
     let result = [];
 
     for (let i = 0; i < selectors.length; i++)
     {
       let selector = selectors[i];
       let key = keys[i];
@@ skipping 86 matching lines @@
     }) =>
     {
       if (Cu.isDeadWrapper(window))
       {
         // We are too late, the window is gone already.
         return;
       }
 
       if (enabled)
       {
-        if (!this.sheet)
-        {
-          this.sheet = Utils.styleService.preloadSheet(this.styleURL,
-              Ci.nsIStyleSheetService.USER_SHEET);
-        }
-
         let utils = window.QueryInterface(Ci.nsIInterfaceRequestor)
                           .getInterface(Ci.nsIDOMWindowUtils);
-        try
+
+        let specificOnly = filter;

[kzar, 2016/10/07 09:30:21]

I don't understand this line, isn't filter being given as filter.text by
elemHideFF.js for "elemhideEnabled"? How can we use the filter.text for
specificOnly?

[Wladimir Palant, 2016/10/07 09:37:49]

There are two kinds of filters that we can get in the response -
$elemhide/$document filters (disable element hiding altogether) or $generichide
filters (disable only generic filters). If enabled is true then it cannot be the
former. So if we have a filter at this point then a $generichide filter applies.

Not sure how this can be made more obvious, maybe send true/false/"specificonly"
for enabled? Combining boolean and string in the same field seems ugly. And I
don't really want to add more data to be sent.

[kzar, 2016/10/07 09:53:31]

Ah, I finally wrapped my head around it :). Well I suppose `let specificOnly =
!!filter;` might be a bit clearer? I guess a small comment here could also help
explain the thinking.

(I wonder why we need to check the contents of `filter` at all, won't it always
be truthy if enabled is true?)

[kzar, 2016/10/07 10:15:29]

(OK, I missed the final `return {enabled: true};` before. Now I see why you need
to check filter.)

[Wladimir Palant, 2016/10/07 10:53:05]

Done.

+        if (!specificOnly)
         {
-          utils.addSheet(this.sheet, Ci.nsIStyleSheetService.USER_SHEET);
-        }
-        catch (e)
-        {
-          // Ignore NS_ERROR_ILLEGAL_VALUE - it will be thrown if we try to add
-          // the stylesheet multiple times to the same document (the observer
-          // will be notified twice for some documents).
-          if (e.result != Cr.NS_ERROR_ILLEGAL_VALUE)
-            throw e;
+          if (!this.sheet)
+          {
+            this.sheet = Utils.styleService.preloadSheet(this.styleURL,
+                Ci.nsIStyleSheetService.USER_SHEET);
+          }
+
+          try
+          {
+            utils.addSheet(this.sheet, Ci.nsIStyleSheetService.USER_SHEET);
+          }
+          catch (e)
+          {
+            // Ignore NS_ERROR_ILLEGAL_VALUE - it will be thrown if we try to add
+            // the stylesheet multiple times to the same document (the observer
+            // will be notified twice for some documents).
+            if (e.result != Cr.NS_ERROR_ILLEGAL_VALUE)
+              throw e;
+          }
         }
 
         let host = subject.location.hostname;
         if (host)
         {
           try
           {
-            utils.loadSheetUsingURIString(this.styleURL.spec + "=" +
-                encodeURIComponent(host), Ci.nsIStyleSheetService.USER_SHEET);
+            let suffix = "=" + encodeURIComponent(host);
+            if (specificOnly)
+              suffix += "&specificonly";
+            utils.loadSheetUsingURIString(this.styleURL.spec + suffix,
+                Ci.nsIStyleSheetService.USER_SHEET);
           }
           catch (e)
           {
             // Ignore NS_ERROR_ILLEGAL_VALUE - it will be thrown if we try to add
             // the stylesheet multiple times to the same document (the observer
             // will be notified twice for some documents).
             if (e.result != Cr.NS_ERROR_ILLEGAL_VALUE)
               throw e;
           }
         }
       }
-      else if (filter)
+
+      if (filter)
       {
         RequestNotifier.addNodeData(window.document, window.top, {
           contentType, docDomain, thirdParty, location, filter, filterType
         });
       }
     });
   }
 };
 observer.init();