From 5e4d0b6e1a319c0a329ba967f529e4b63b1c70f3 Mon Sep 17 00:00:00 2001
From: Bianca Danforth <biancadanforth@gmail.com>
Date: Fri, 2 Aug 2019 20:06:00 -0700
Subject: [PATCH]  #118: Fall back to next best sync solution

Unfortunately, the previous commit's approach based on clickability proved untenable.[1]

A distant second sync solution is to early return where possible and use the cheapest styles first (the second implementation approach[2]), which reduces Fathom jank by 13%*, though there are a couple differences:
* No 'getBoxQuads'. This is an experimental API only enabled on Nightly.
* Checks if the element is off-screen. The Price Tracker implementation was missing this check.

Unfortunately, this implementation still uses 'ancestors', which causes expensive XRays[3] work in extension applications and still triggers layout flushes at suboptimal times. This is something that can be avoided with an async solution to the tune of a 40% reduction in jank using 'requestAnimationFrame' and 'setTimeout'[4].

On the brighter side, it is more correct than the previous implementation, removing 'getComputedStyle().width' and 'getComputedStyle().height' completely and covering more valid cases than before.

*: This is slightly worse than the expected 16%, because my original implementation in Price Tracker did not check for elements off-screen as the Fathom implementation does. Its profile[5] shows:
 - The largest unresponsive chunk is still caused by Fathom extraction, contributing 399 ms of jank right around page load.
 - `isVisible` made up 238 ms (60%) of this jank.
 - This change reduced overall Fathom-related jank by 61 ms (13%) compared to the original implementation of isVisible[2].

[1]: https://github.com/mozilla/fathom/pull/116#discussion_r309410670
[2]: https://github.com/mozilla/price-tracker/issues/319#issue-468970781
[3]: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/Xray_vision
[4]: https://github.com/mozilla/price-tracker/issues/319#issuecomment-517554826
[5]: https://perfht.ml/2T0oYQS
---
 utilsForFrontend.mjs | 51 +++++++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/utilsForFrontend.mjs b/utilsForFrontend.mjs
index c1defc96..00d21641 100644
--- a/utilsForFrontend.mjs
+++ b/utilsForFrontend.mjs
@@ -458,30 +458,47 @@ export function sigmoid(x) {
 }
 
 /**
- * Return whether an element is practically visible, considing things like 0
- * size or opacity, and clickability.
+ * Return whether an element is practically visible, considering things like 0
+ * size or opacity, ``visibility: hidden`` and ``overflow: hidden``.
+ *
+ * This could be 5x more efficient if https://github.com/w3c/csswg-drafts/issues/4122
+ * happens.
  */
 export function isVisible(fnodeOrElement) {
     const element = toDomElement(fnodeOrElement);
-    const rect = element.getBoundingClientRect();
-    if (rect.width === 0 || rect.height === 0) {
+    // Avoid reading ``display: none`` due to Bug 1381071
+    const elementRect = element.getBoundingClientRect();
+    if (elementRect.width === 0 && elementRect.height === 0) {
         return false;
     }
-    const style = getComputedStyle(element);
-    if (style.opacity === '0') {
+    const elementStyle = getComputedStyle(element);
+    if (elementStyle.visibility === 'hidden') {
         return false;
     }
-    // workaround for https://github.com/w3c/csswg-drafts/issues/4122
-    const scrollX = window.pageXOffset;
-    const scrollY = window.pageYOffset;
-    const absX = rect.x + scrollX;
-    const absY = rect.y + scrollY;
-    window.scrollTo(absX, absY);
-    const newX = absX - window.pageXOffset;
-    const newY = absY - window.pageYOffset;
-    const eles = document.elementsFromPoint(newX, newY);
-    window.scrollTo(scrollX, scrollY);
-    return eles.includes(element);
+    const frame = element.ownerDocument.defaultView;
+    for (const ancestor of ancestors(element)) {
+        const isElement = ancestor === element;
+        const style = isElement ? elementStyle : getComputedStyle(ancestor);
+        if (style.opacity === '0') {
+            return false;
+        }
+        if (style.display === 'contents') {
+            // display: contents elements have no box themselves, but children are
+            // still rendered.
+            continue;
+        }
+        const rect = isElement ? elementRect : ancestor.getBoundingClientRect();
+        if ((rect.width === 0 || rect.height === 0) && elementStyle.overflow === 'hidden') {
+            // Zero-sized ancestors don’t make descendants hidden unless the descendant
+            // has overflow: hidden
+            return false;
+        }
+        // Check if the element is off-screen
+        if (isElement && ((rect.right + frame.scrollX < 0) || (rect.bottom + frame.scrollY < 0))) {
+            return false;
+        }
+    }
+    return true;
 }
 
 /**