Issue 7450 - Implement hide-if-contains-visible-text snippet

lib/content/snippets.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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 486 matching lines @@
     if (color == "rgba(0, 0, 0, 0)")
       return false;
     if (style.getPropertyValue("background-color") == color)
       return false;
 
     return true;
   }
 
   /**
    * Check if an element is visible
-   * @param {Element} element The element to check visibility of/
+   * @param {Element} element The element to check visibility of.
    * @param {?CSSStyleDeclaration} style The computed style of element. If
    *   falsey it will be queried.
    * @param {?Element} closest The closest parent to reach.
    * @return {bool} Whether the element is visible.
    */
   function isVisible(element, style, closest)
   {
     if (!style)
       style = window.getComputedStyle(element);
 
     if (style.getPropertyValue("display") == "none")
       return false;
     let visibility = style.getPropertyValue("visibility");
     if (visibility == "hidden" || visibility == "collapse")
       return false;
 
     if (!closest || element == closest)
       return true;
 
-    return isVisible(element.parentNode, null, closest);

[a.giammarchi, 2019/05/08 09:43:47]

I have bad feelings about this for at least two reasons:

 1. it breaks if it reaches the `document`, since no `parentElement` would be
there
 2. it's very expensive in complex DOM

I have a coupe of suggestions here:

 1. how about we cut the mustard and use element.getBoundingClientRect()
directly, so that if either width or height are 0 we can call it a day without
crawling to the root each node that is visible?
 2. if gBCR() is not a solution, mind checking that `parentNode` is a `nodeType`
=== `Node.ELEMENT_NODE` so that stuff in fragments, or document itself, wouldn't
break?

[hub, 2019/05/08 17:03:42]

I'll stop if parent is null.
How come? it is already limited to closest. I realise I should have used
`parentElement` instead.
To "closest"
Using `parentElement` now and stop when it returns null

[a.giammarchi, 2019/05/09 08:58:59]

where does this happen? 'cause all I see is that you grab the computed style
right away from an element without checking that element is live/not null

I guess logically, all I am saying is that if !closest || element == closest ||
!element.parentElement you should return and avoid recursion.

Please note I am talking about the signature as is, where one could pass closest
as plain `document` simply to know if a generic element is visible.

In that case:

  * closest is document
  * element is anything
  * the moment element crawl up to documentElement, closest is there but no
parentElement is returned so that the following breaks:

window.getComputedStyle(document.documentElement.parentElement)

[hub, 2019/05/09 12:06:26]

It is actually better if I actually properly submit the changes for review.
Sorry about that.

+    let parent = element.parentElement;
+    if (!parent)
+      return true;
+
+    return isVisible(parent, null, closest);
   }
 
   /**
    * Returns the visible text content from an element and its descendants.
    * @param {Element} element The element whose visible text we want.
    * @param {Element} closest The closest parent to reach while checking
    *   for visibility.
    * @returns {string} The text that is visible.
    */
   function getVisibleContent(element, closest)
@@ skipping 478 matching lines @@
 
       args[0] = url.href;
     }
 
     return fetch_.apply(this, args);
   };
 }
 
 exports["strip-fetch-query-parameter"] = makeInjector(stripFetchQueryParameter,
                                                       toRegExp, regexEscape);