Skip to content

Commit

Permalink
#118: Fall back to next best sync solution
Browse files Browse the repository at this point in the history
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]: #116 (comment)
[2]: mozilla/price-tracker#319 (comment)
[3]: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/Xray_vision
[4]: mozilla/price-tracker#319 (comment)
[5]: https://perfht.ml/2T0oYQS
  • Loading branch information
biancadanforth committed Aug 3, 2019
1 parent 6d8c030 commit 5e4d0b6
Showing 1 changed file with 34 additions and 17 deletions.
51 changes: 34 additions & 17 deletions utilsForFrontend.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down

0 comments on commit 5e4d0b6

Please sign in to comment.