From 7115cfbea5e2e8ce76026f88819cfb50dc806caf Mon Sep 17 00:00:00 2001 From: girst Date: Tue, 29 Aug 2023 19:04:12 +0200 Subject: [PATCH] (half/full) page scrolling: don't take position:absolute headers into account Github's new code view[1] places each line of code visible on screen, as well as some above and below, as a position:absolute element (within a position:relative parent). Our code did not check whether such an element is absolute w.r.t the current scrolling viewport or some other element and would horribly miscalculate the amount to scroll: It would take all off-screen position:absolute elements and add their heights, then subtract that from the amount to scroll; even turning the scoll amount negative! Further, * medium[.]com does not appear to use any footer any more, and * the website mentioned in #698, inbound[.]org no longer resolves. The referenced Firefox implementation doesn't check position:absolute elements at all. As an actual improvement, Firefox now actually checks for position:sticky elements, but only when they are 'stuck' (and therefore behaving like position:fixed)[2]. They use some Gecko internals, but this could be implemented in VimFx using something along the lines of this: isStuck = (elem) => elem.getBoundingClientRect().top <= parseInt(getComputedStyle(elem).top) 1: https://github.blog/2023-06-21-crafting-a-better-faster-code-view/ 2: https://hg.mozilla.org/mozilla-central/rev/263936aecc1d --- extension/lib/viewport.coffee | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/extension/lib/viewport.coffee b/extension/lib/viewport.coffee index 808db27..9a45753 100644 --- a/extension/lib/viewport.coffee +++ b/extension/lib/viewport.coffee @@ -10,8 +10,6 @@ getPosition = (element) -> isFixed = (element) -> getPosition(element) == 'fixed' -isFixedOrAbsolute = (element) -> getPosition(element) in ['fixed', 'absolute'] - adjustRectToViewport = (rect, viewport) -> # The right and bottom values are subtracted by 1 because # `document.elementFromPoint(right, bottom)` does not return the element @@ -178,15 +176,15 @@ getFixedHeaderAndFooter = (window) -> for candidate in candidates rect = candidate.getBoundingClientRect() continue unless rect.height <= maxHeight and rect.width >= minWidth - # Checking for `position: fixed;` or `position: absolute;` is the absolutely - # most expensive operation, so that is done last. + # Checking for `position: fixed;` is the absolutely most expensive + # operation, so that is done last. switch when rect.top <= headerBottom and rect.bottom > headerBottom and - isFixedOrAbsolute(candidate) + isFixed(candidate) header = candidate headerBottom = rect.bottom when rect.bottom >= footerTop and rect.top < footerTop and - isFixedOrAbsolute(candidate) + isFixed(candidate) footer = candidate footerTop = rect.top -- 2.39.3