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

lib/child/cssProperties.js

Index: lib/child/cssProperties.js
===================================================================
--- a/lib/child/cssProperties.js
+++ b/lib/child/cssProperties.js
@@ -17,17 +17,18 @@
 
 "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"
@@ -55,24 +56,52 @@
     {
       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);
     };