diff options
| author | Stephen Blott | 2015-02-08 07:39:20 +0000 |
|---|---|---|
| committer | Stephen Blott | 2015-02-08 07:57:04 +0000 |
| commit | a656b1f52c6879bb9b4468bbaffe7664e48926c0 (patch) | |
| tree | c0322ddcce7443a71ed88a3f8aa315271ac040f6 /content_scripts/scroller.coffee | |
| parent | 8dd70aaa6c1044c5c8b389e61d67b72d234432d0 (diff) | |
| download | vimium-a656b1f52c6879bb9b4468bbaffe7664e48926c0.tar.bz2 | |
Temporary fix for obscure scrolling bug.
I'm seeing a strange scrolling bug on some pages, for example, here:
http://www.steephill.tv/tour-of-qatar/.
Background:
When we scroll, we first have to decide which element to scroll.
Because of a chrome bug, we do this by testing candidate elements:
scroll the element up by one px, then back down again. If it moved,
then it's scrollable, and it's the element we should scroll.
Bug:
In some cases, the micro-scroll-up succeeds, but the subsequent
micro-scroll-down fails. It's not clear how this could happen. It
may be a chrome bug. In any case, as a result, we conclude that the
element is not scrollable, and proceed to the next candidate. We end
up finding no scrollable element, and j/k scrolling is broken. If
you press-and-hold "j" or "k", you can see the repeated
micro-scroll-ups.
Problem:
I can't reproduce this in a clean chrome account. I only see it in
my live chrome instance. I've tried disabling all extensions, but
the problem persists. So there's something else at play, but I
haven't yet been able to track it down.
Solution:
This commit is a temporary workaround. We just default to scrolling
document.body if we can't find anything else to scroll. Most of the
time, that's the right thing to do. And it's what we would have done
prior to implementing smooth scrolling. Nevertheless, it isn't a
great solution...
Notes:
It's possible that this is related to #1117. However, users there
have reported *not* seeing the micro-scrolls. So probably not.
I'm not posting this as a PR, because I can't reliably reproduce the
bug. However, the fix pretty much does what we did before smooth
scrolling, it's almost certainly safe. So I'm just pushing this
straight into master.
Diffstat (limited to 'content_scripts/scroller.coffee')
| -rw-r--r-- | content_scripts/scroller.coffee | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee index 6e2e1ffc..fbc34794 100644 --- a/content_scripts/scroller.coffee +++ b/content_scripts/scroller.coffee @@ -224,7 +224,7 @@ Scroller = window.scrollBy(0, amount) return - activatedElement ||= document.body and firstScrollableElement() + activatedElement ||= (document.body and firstScrollableElement()) or document.body return unless activatedElement # Avoid the expensive scroll calculation if it will not be used. This reduces costs during smooth, @@ -235,7 +235,7 @@ Scroller = CoreScroller.scroll element, direction, elementAmount scrollTo: (direction, pos) -> - activatedElement ||= document.body and firstScrollableElement() + activatedElement ||= (document.body and firstScrollableElement()) or document.body return unless activatedElement element = findScrollableElement activatedElement, direction, pos, 1 |