Skip to content

Commit

Permalink
#118: Change isVisible to check for clickability to improve performance
Browse files Browse the repository at this point in the history
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].

Notes:
* If the proposal to the W3C CSS Working Group (see inline comment in patch) is implemented, this clickability approach could forgo the workaround and see as much as 81% (374 ms) reduction in Fathom-related jank[3].
* This implementation can still benefit from memoization, as the same element (e.g. 'div') could be considered for multiple different 'type's[6].

[1]: https://perfht.ml/30wkWT7
[2]: https://perfht.ml/2Y5FCQ1
[3]: mozilla/price-tracker#319
[4]: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/Xray_vision"
[6]: https://mozilla.github.io/fathom/glossary.html
  • Loading branch information
biancadanforth committed Aug 9, 2019
1 parent 68ce4cd commit 335657e
Showing 1 changed file with 19 additions and 20 deletions.
39 changes: 19 additions & 20 deletions utilsForFrontend.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down

0 comments on commit 335657e

Please sign in to comment.