gblazex/smoothscroll

All scrolling controlled by SmoothScroll works only for a few pixels on sites like this one.

Closed this issue · 2 comments

DrReD commented

All scrolling controlled by SmoothScroll works only for a few pixels on sites like this one:
https://shkspr.mobi/blog/2024/01/whats-the-smallest-file-size-for-a-1-pixel-image/

It may be triggered by HTML/CSS alone, because it still happens with everything else disabled (by UBlock Origin)

Thanks for the great extension!

The issue

for reference the site has an uncleared float:right in body::after pseudo element style

And that causes our overflow script to return body instead of html as the scrollable element.
But HTML has a scroll-behavior:smooth style, which we do normally control for, but in this case we got returned the body so we apply it to the wrong element.

Unless you can find other websites with similar problems, I don't want to add complexity to the code.

Why no fix for now

As you can see we are already dealing with a lot of cases (this doesn't even include the scroll-behavior fixes)

//  (body)                (root)
//         | hidden | visible | scroll |  auto  |
// hidden  |   no   |    no   |   YES  |   YES  |
// visible |   no   |   YES   |   YES  |   YES  |
// scroll  |   no   |   YES   |   YES  |   YES  |
// auto    |   no   |   YES   |   YES  |   YES  |

function overflowingAncestor(el, isX) {
    var elems = [];
    var body = document.body;
    var rootScrollHeight = root.scrollHeight;
    var rootScrollWidth  = root.scrollWidth;
    while (el) {
        var cached = getCache(el, isX);
        if (cached) {
            return setCache(elems, cached, isX);
        }
        elems.push(el);
        if (isX && rootScrollWidth  === el.scrollWidth ||
           !isX && rootScrollHeight === el.scrollHeight) {
            var topOverflowsNotHidden = overflowNotHidden(root, isX) && overflowNotHidden(body, isX);
            var isOverflowCSS = topOverflowsNotHidden || overflowAutoOrScroll(root, isX);
            if (isFrame && isContentOverflowing(root, isX) || 
               !isFrame && isOverflowCSS) {
                return setCache(elems, getScrollRoot(), isX); 
            }
        } else if (isContentOverflowing(el, isX) && overflowAutoOrScroll(el, isX)) {
            return setCache(elems, el, isX);
        }
        // Support shadow DOM
        el = el.parentElement || (el.getRootNode && el.getRootNode().host); 
    }
}

function isContentOverflowing(el, isX) {
    return isX ? (el.clientWidth  + 10 < el.scrollWidth) 
               : (el.clientHeight + 10 < el.scrollHeight);
}

function computedOverflow(el, isX) {
    var property = isX ? 'overflow-x' : 'overflow-y';
    return getComputedStyle(el, '').getPropertyValue(property);
}

// typically for <body> and <html>
function overflowNotHidden(el, isX) {
    return (computedOverflow(el, isX) != 'hidden');
}

// for all other elements
function overflowAutoOrScroll(el, isX) {
    return /^(scroll|auto)$/.test(computedOverflow(el, isX));
}

more debug results

with problematic style:
[document.body.clientHeight, document.body.scrollHeight]
(2) [4414, 4456]  // <- isContentOverflowing returns true for body, it "looks" scrollable

without problematic style:
[document.body.clientHeight, document.body.scrollHeight]
(2) [4448, 4456]

How to fix (if we need to later)

Potential fixes to consider (if we find many other sites with this issue)

a) prioritize html over body if both overflow. Before returning body, at least do a check on html too

b) if we happen to do scroll-behavior temporary disabling on body, make sure to always do it on html too, just to be safe

found a fix by

element.scrollBy({ behavior:'instant' })

will be fixed in new version