From 4855173ec4e8e9852c4a6d51be0a8a5519e9daba Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Mon, 26 Dec 2016 06:17:30 +0000
Subject: Fix (another) infinite-scroll issue.

Steps to reproduce:

- Press and hold `j`.
- Tab `k`.
- Release `j`.

The tap on `k` is uninstalling the `cancelEventListener` installed by
`j`, and because we advance `@time`, the `j` stops scrolling and
removes the `cancelEventListener` installed for `k`.  So we end up with
no `cancelEventListener` installed.

The fix is to make the `cancelEventListener` specific to the scroller
instance.

The more fundamental problem here is that we're mixing dynamic and
static state.  A better approach would be to have `CoreScroller` as a
class, with a new instance created for each scroll instance.
---
 content_scripts/scroller.coffee | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

(limited to 'content_scripts')

diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee
index a533c161..b4a0d799 100644
--- a/content_scripts/scroller.coffee
+++ b/content_scripts/scroller.coffee
@@ -128,15 +128,15 @@ checkVisibility = (element) ->
 CoreScroller =
   init: ->
     @time = 0
-    @lastEvent = @keyIsDown = @cancelEventListenerId = null
+    @lastEvent = @keyIsDown = null
+    @installCanceEventListener()
 
   # This installs listeners for events which should cancel smooth scrolling.
   installCanceEventListener: ->
-    @removeCancelEventListener()
     # NOTE(smblott) With extreme keyboard configurations, Chrome sometimes does not get a keyup event for
     # every keydown, in which case tapping "j" scrolls indefinitely.  This appears to be a Chrome/OS/XOrg bug
     # of some kind.  See #1549.
-    @cancelEventListenerId = handlerStack.push
+    handlerStack.push
       _name: 'scroller/track-key-status'
       keydown: (event) =>
         handlerStack.alwaysContinueBubbling =>
@@ -151,10 +151,6 @@ CoreScroller =
         handlerStack.alwaysContinueBubbling =>
           @time += 1 if event.target == window
 
-  removeCancelEventListener: ->
-    handlerStack.remove @cancelEventListenerId if @cancelEventListenerId?
-    @cancelEventListenerId = @lastEvent = @keyIsDown = null
-
   # Return true if CoreScroller would not initiate a new scroll right now.
   wouldNotInitiateScroll: -> @lastEvent?.repeat and Settings.get "smoothScroll"
 
@@ -194,6 +190,7 @@ CoreScroller =
     totalElapsed = 0.0
     calibration = 1.0
     previousTimestamp = null
+    cancelEventListener = @installCanceEventListener()
 
     animate = (timestamp) =>
       previousTimestamp ?= timestamp
@@ -221,7 +218,7 @@ CoreScroller =
         requestAnimationFrame animate
       else
         # We're done.
-        @removeCancelEventListener()
+        handlerStack.remove cancelEventListener
         checkVisibility element
 
     # If we've been asked not to be continuous, then we advance time, so the myKeyIsStillDown test always
@@ -229,7 +226,6 @@ CoreScroller =
     ++@time unless continuous
 
     # Start scrolling.
-    @installCanceEventListener()
     requestAnimationFrame animate
 
 # Scroller contains the two main scroll functions which are used by clients.
-- 
cgit v1.2.3