From a7114714a4dd9551d41dfdfaa2025f1693fcf001 Mon Sep 17 00:00:00 2001 From: Bianca Danforth Date: Wed, 24 Jul 2019 13:22:06 -0700 Subject: [PATCH] #118: Change isVisible to check for clickability to improve performance Compared to a baseline profile[1] of Price Tracker's current 'isVisible' implementation (on which Fathom's 'isVisible' method is based), this clickability approach offers a 53% (146 ms) reduction in Fathom-related jank[2] for the same locally hosted sample page. This is largely due to removing the use of the 'ancestors' Fathom method in 'isVisible'[3], which was performing a lot of redundant layout accesses (and triggering a lot of layout flushes) for the same elements. Also, at least in an extension application, DOM accesses (e.g. repeatedly getting the next 'parentNode' in 'ancestors') are very expensive due to X-Rays[4]. It should be noted that this implementation can still benefit from memoization, as the same element (e.g. 'div') could be considered for multiple different 'type's[5]. [1]: https://perfht.ml/30wkWT7 [2]: https://perfht.ml/2Y5FCQ1 [3]: https://github.com/mozilla/price-tracker/issues/319 [4]: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/Xray_vision" [5]: https://mozilla.github.io/fathom/glossary.html --- utilsForFrontend.mjs | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/utilsForFrontend.mjs b/utilsForFrontend.mjs index 94f208dd..c1defc96 100644 --- a/utilsForFrontend.mjs +++ b/utilsForFrontend.mjs @@ -459,30 +459,29 @@ export function sigmoid(x) { /** * Return whether an element is practically visible, considing things like 0 - * size or opacity, ``display: none``, and ``visibility: hidden``. + * size or opacity, and clickability. */ export function isVisible(fnodeOrElement) { const element = toDomElement(fnodeOrElement); - for (const ancestor of ancestors(element)) { - const style = getComputedStyle(ancestor); - if (style.visibility === 'hidden' || - style.display === 'none' || - style.opacity === '0' || - style.width === '0px' || - style.height === '0px') { - return false; - } else { - // It wasn't hidden based on a computed style. See if it's - // offscreen: - const rect = element.getBoundingClientRect(); - const frame = element.ownerDocument.defaultView; // window or iframe - if ((rect.right + frame.scrollX < 0) || - (rect.bottom + frame.scrollY < 0)) { - return false; - } - } + const rect = element.getBoundingClientRect(); + if (rect.width === 0 || rect.height === 0) { + return false; + } + const style = getComputedStyle(element); + if (style.opacity === '0') { + return false; } - return true; + // 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); } /**