Issue 3969 - Hits are not counted for custom CSS property rules

lib/child/cssProperties.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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
 (function()
 {
   let {Services} = Cu.import("resource://gre/modules/Services.jsm", {});
 
   let {port} = require("messaging");
-  let {getFrames} = require("child/utils");
+  let {getFrames, isPrivate} = require("child/utils");
+  let {RequestNotifier} = require("child/requestNotifier");
 
   function getFilters(window, callback)
   {
     let message = {
       frames: getFrames(window),
       payload: {
         type: "filters.get",
         what: "cssproperties"
       }
     };
@@ skipping 17 matching lines @@
 
     let onContentWindow = (subject, topic, data) =>
     {
       if (!(subject instanceof Ci.nsIDOMWindow))
         return;
 
       let onReady = event =>
       {
         subject.removeEventListener("load", onReady);
         let handler = new scope.CSSPropertyFilters(
-          subject, getFilters.bind(null, subject), selectors =>
+          subject, getFilters.bind(null, subject), (selectors, filters) =>
           {
             if (selectors.length == 0)
               return;
 
             addUserCSS(subject, selectors.map(
               selector => selector + "{display: none !important;}"
             ).join("\n"));
+
+            if (!isPrivate(subject))
+              port.emit("addHits", filters);
+
+            let docDomain = null;
+            try
+            {
+              // We are calling getFrames() here because it will consider
+              // "inheritance" for about:blank and data: frames.
+              docDomain = new URL(getFrames(subject)[0].location).hostname;
+            }
+            catch (e)
+            {
+              // Invalid URL?

[saroyanm, 2016/06/10 08:55:11]

Detail: I'm not sure if this comment provides any value.

[Wladimir Palant, 2016/06/10 14:59:54]

Well, eslint objects to empty catch clauses. In fact, the comment explains the
error condition that we are ignoring here.

+            }
+
+            for (let filter of filters)
+            {
+              RequestNotifier.addNodeData(subject.document, subject.top, {
+                contentType: "ELEMHIDE",
+                docDomain: docDomain,
+                thirdParty: false,
+                // TODO: Show the actual matching selector here?

[saroyanm, 2016/06/10 08:55:11]

Detail: Feels like you forgot "TODO" Smth, or maybe remove the comment

[Wladimir Palant, 2016/06/10 14:59:54]

No, I meant to have this TODO comment here. Addressing it requires changes to
adblockpluscore meaning that I cannot do it right now.

[saroyanm, 2016/06/10 17:00:24]

Acknowledged.

+                location: filter.replace(/^.*?##/, ""),

[saroyanm, 2016/06/10 08:55:12]

Why do we remove the URL part from the filter to assign to location ? Shouldn't
we assigne location/URL ? Currently in the "Address" column we are showing the
"Property selector" part of the filter.

[Wladimir Palant, 2016/06/10 14:59:54]

hit.location is normally the address that the filter matched, not the domain of
the document (we have hit.docDomain for that already). For element hiding
filters there is no address however which is why we show the selector part of
the filter instead (as it represents the matching element in a way). For CSS
property filters we duplicated this behavior for now, we could improve it
however by showing the *actual* selector rather than the selector part of the
filter - hence the TODO comment.

[saroyanm, 2016/06/10 17:00:24]

Acknowledged.

+                filter: filter,
+                filterType: "cssproperty"
+              });
+            }
           }
         );
 
         handler.load(() => handler.apply());
       };
 
       subject.addEventListener("load", onReady);
     };
 
     Services.obs.addObserver(onContentWindow, "content-document-global-created",
         false);
     onShutdown.add(() =>
     {
       Services.obs.removeObserver(onContentWindow,
           "content-document-global-created");
     });
   }
 
   initCSSPropertyFilters();
 })();