Factor out scrolling code into a new file.

Also fix a bunch of div-scrolling behavior. Closes #486.

5 files changed, 112 insertions, 61 deletions

diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee
new file mode 100644
index 00000000..08ab14a1
--- /dev/null
+++ b/content_scripts/scroller.coffee
@@ -0,0 +1,89 @@
+window.Scroller = root = {}
+
+#
+# activatedElement is different from document.activeElement -- the latter seems to be reserved mostly for
+# input elements. This mechanism allows us to decide whether to scroll a div or to scroll the whole document.
+#
+activatedElement = null
+
+root.init = ->
+  handlerStack.push DOMActivate: -> activatedElement = event.target
+
+scrollProperties =
+  x: {
+    axisName: 'scrollLeft'
+    max: 'scrollWidth'
+    viewSize: 'clientHeight'
+  }
+  y: {
+    axisName: 'scrollTop'
+    max: 'scrollHeight'
+    viewSize: 'clientWidth'
+  }
+
+getDimension = (el, direction, name) ->
+  # the clientSizes of the body are the dimensions of the entire page, but the viewport should only be the
+  # part visible through the window
+  if name is 'viewSize' and el is document.body
+    if direction is 'x' then window.innerWidth else window.innerHeight
+  else
+    el[scrollProperties[direction][name]]
+
+# Chrome does not report scrollHeight accurately for nodes with pseudo-elements of height 0 (bug 110149).
+# Therefore we cannot figure out if we have scrolled to the bottom of an element by testing if scrollTop +
+# clientHeight == scrollHeight. So just try to increase scrollTop blindly -- if it fails we know we have
+# reached the end of the content.
+ensureScrollChange = (direction, changeFn) ->
+  axisName = scrollProperties[direction].axisName
+  element = activatedElement
+  loop
+    oldScrollValue = element[axisName]
+    changeFn(element, axisName)
+    lastElement = element
+    # we may have an orphaned element. if so, just scroll the body element.
+    element = element.parentElement || document.body
+    break unless (lastElement[axisName] == oldScrollValue && lastElement != document.body)
+
+  # if the activated element has been scrolled completely offscreen, subsequent changes in its scroll
+  # position will not provide any more visual feedback to the user. therefore we deactivate it so that
+  # subsequent scrolls only move the parent element.
+  rect = activatedElement.getBoundingClientRect()
+  if (rect.bottom < 0 || rect.top > window.innerHeight || rect.right < 0 || rect.left > window.innerWidth)
+    activatedElement = lastElement
+
+# scroll the active element in :direction by :amount * :factor.
+# :factor is needed because :amount can take on string values, which scrollBy converts to element dimensions.
+root.scrollBy = (direction, amount, factor = 1) ->
+  # if this is called before domReady, just use the window scroll function
+  if (!document.body and amount instanceof Number)
+    if (direction == "x")
+      window.scrollBy(amount, 0)
+    else
+      window.scrollBy(0, amount)
+    return
+
+  if (!activatedElement || !isRendered(activatedElement))
+    activatedElement = document.body
+
+  amount = getDimension activatedElement, direction, amount if Utils.isString amount
+
+  amount *= factor
+
+  if (amount != 0)
+    ensureScrollChange direction, (element, axisName) -> element[axisName] += amount
+
+root.scrollTo = (direction, pos) ->
+  return unless document.body
+
+  if (!activatedElement || !isRendered(activatedElement))
+    activatedElement = document.body
+
+  pos = getDimension activatedElement, direction, pos if Utils.isString pos
+
+  ensureScrollChange direction, (element, axisName) -> element[axisName] = pos
+
+# TODO refactor and put this together with the code in getVisibleClientRect
+isRendered = (element) ->
+  computedStyle = window.getComputedStyle(element, null)
+  return !(computedStyle.getPropertyValue("visibility") != "visible" ||
+      computedStyle.getPropertyValue("display") == "none")
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index a656eb70..044b0a49 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -4,20 +4,20 @@
 # background page that we're in domReady and ready to accept normal commands by connectiong to a port named
 # "domReady".
 #
+window.handlerStack = new HandlerStack
+
 insertModeLock = null
 findMode = false
 findModeQuery = { rawQuery: "" }
 findModeQueryHasResults = false
 findModeAnchorNode = null
 isShowingHelpDialog = false
-handlerStack = new HandlerStack
 keyPort = null
 # Users can disable Vimium on URL patterns via the settings page.
 isEnabledForUrl = true
 # The user's operating system.
 currentCompletionKeys = null
 validFirstKeys = null
-activatedElement = null
 
 # The types in <input type="..."> that we consider for focusInput command. Right now this is recalculated in
 # each content script. Alternatively we could calculate it once in the background page and use a request to
@@ -96,6 +96,8 @@ initializePreDomReady = ->
   settings.addEventListener("load", LinkHints.init.bind(LinkHints))
   settings.load()
 
+  Scroller.init()
+
   checkIfEnabledForUrl()
 
   refreshCompletionKeys()
@@ -187,7 +189,7 @@ enterInsertModeIfElementIsFocused = ->
   if (document.activeElement && isEditable(document.activeElement) && !findMode)
     enterInsertModeWithoutShowingIndicator(document.activeElement)
 
-onDOMActivate = (event) -> activatedElement = event.target
+onDOMActivate = (event) -> handlerStack.bubbleEvent 'DOMActivate', event
 
 executePageCommand = (request) ->
   return unless frameId == request.frameId
@@ -199,50 +201,6 @@ executePageCommand = (request) ->
 
   refreshCompletionKeys(request)
 
-#
-# activatedElement is different from document.activeElement -- the latter seems to be reserved mostly for
-# input elements. This mechanism allows us to decide whether to scroll a div or to scroll the whole document.
-#
-scrollActivatedElementBy = (direction, amount) ->
-  # if this is called before domReady, just use the window scroll function
-  if (!document.body)
-    if (direction == "x")
-      window.scrollBy(amount, 0)
-    else
-      window.scrollBy(0, amount)
-    return
-
-  # TODO refactor and put this together with the code in getVisibleClientRect
-  isRendered = (element) ->
-    computedStyle = window.getComputedStyle(element, null)
-    return !(computedStyle.getPropertyValue("visibility") != "visible" ||
-        computedStyle.getPropertyValue("display") == "none")
-
-  if (!activatedElement || !isRendered(activatedElement))
-    activatedElement = document.body
-
-  scrollName = if (direction == "x") then "scrollLeft" else "scrollTop"
-
-  # Chrome does not report scrollHeight accurately for nodes with pseudo-elements of height 0 (bug 110149).
-  # Therefore we just try to increase scrollTop blindly -- if it fails we know we have reached the end of the
-  # content.
-  if (amount != 0)
-    element = activatedElement
-    loop
-      oldScrollValue = element[scrollName]
-      element[scrollName] += amount
-      lastElement = element
-      # we may have an orphaned element. if so, just scroll the body element.
-      element = element.parentElement || document.body
-      break unless (lastElement[scrollName] == oldScrollValue && lastElement != document.body)
-
-  # if the activated element has been scrolled completely offscreen, subsequent changes in its scroll
-  # position will not provide any more visual feedback to the user. therefore we deactivate it so that
-  # subsequent scrolls only move the parent element.
-  rect = activatedElement.getBoundingClientRect()
-  if (rect.bottom < 0 || rect.top > window.innerHeight || rect.right < 0 || rect.left > window.innerWidth)
-    activatedElement = lastElement
-
 setScrollPosition = (scrollX, scrollY) ->
   if (scrollX > 0 || scrollY > 0)
     DomUtils.documentReady(-> window.scrollBy(scrollX, scrollY))
@@ -258,19 +216,18 @@ window.focusThisFrame = (shouldHighlight) ->
     setTimeout((-> document.body.style.border = borderWas), 200)
 
 extend window,
-  scrollToBottom: -> window.scrollTo(window.pageXOffset, document.body.scrollHeight)
-  scrollToTop: -> window.scrollTo(window.pageXOffset, 0)
-  scrollToLeft: -> window.scrollTo(0, window.pageYOffset)
-  scrollToRight: -> window.scrollTo(document.body.scrollWidth, window.pageYOffset)
-  scrollUp: -> scrollActivatedElementBy("y", -1 * settings.get("scrollStepSize"))
-  scrollDown: ->
-    scrollActivatedElementBy("y", parseFloat(settings.get("scrollStepSize")))
-  scrollPageUp: -> scrollActivatedElementBy("y", -1 * window.innerHeight / 2)
-  scrollPageDown: -> scrollActivatedElementBy("y", window.innerHeight / 2)
-  scrollFullPageUp: -> scrollActivatedElementBy("y", -window.innerHeight)
-  scrollFullPageDown: -> scrollActivatedElementBy("y", window.innerHeight)
-  scrollLeft: -> scrollActivatedElementBy("x", -1 * settings.get("scrollStepSize"))
-  scrollRight: -> scrollActivatedElementBy("x", parseFloat(settings.get("scrollStepSize")))
+  scrollToBottom: -> Scroller.scrollTo "y", "max"
+  scrollToTop: -> Scroller.scrollTo "y", 0
+  scrollToLeft: -> Scroller.scrollTo "x", 0
+  scrollToRight: -> Scroller.scrollTo "x", "max"
+  scrollUp: -> Scroller.scrollBy "y", -1 * settings.get("scrollStepSize")
+  scrollDown: -> Scroller.scrollBy "y", parseFloat(settings.get("scrollStepSize"))
+  scrollPageUp: -> Scroller.scrollBy "y", "viewSize", -1/2
+  scrollPageDown: -> Scroller.scrollBy "y", "viewSize", 1/2
+  scrollFullPageUp: -> Scroller.scrollBy "y", "viewSize", -1
+  scrollFullPageDown: -> Scroller.scrollBy "y", "viewSize"
+  scrollLeft: -> Scroller.scrollBy "x", -1 * settings.get("scrollStepSize")
+  scrollRight: -> Scroller.scrollBy "x", parseFloat(settings.get("scrollStepSize"))
 
 extend window,
   reload: -> window.location.reload()
@@ -464,7 +421,7 @@ onKeydown = (event) ->
   #
   # Subject to internationalization issues since we're using keyIdentifier instead of charCode (in keypress).
   #
-  # TOOD(ilya): Revisit @ Not sure it's the absolute best approach.
+  # TOOD(ilya): Revisit this. Not sure it's the absolute best approach.
   if (keyChar == "" && !isInsertMode() &&
      (currentCompletionKeys.indexOf(KeyboardUtils.getKeyChar(event)) != -1 ||
       isValidFirstKey(KeyboardUtils.getKeyChar(event))))
diff --git a/lib/utils.coffee b/lib/utils.coffee
index a44bc06c..c92fa1ff 100644
--- a/lib/utils.coffee
+++ b/lib/utils.coffee
@@ -108,6 +108,9 @@ Utils =
     else
       Utils.createSearchUrl string
 
+  # detects both literals and dynamically created strings
+  isString: (obj) -> typeof obj == 'string' or obj instanceof String
+
 # This creates a new function out of an existing function, where the new function takes fewer arguments. This
 # allows us to pass around functions instead of functions + a partial list of arguments.
 Function::curry = ->
diff --git a/manifest.json b/manifest.json
index ea6e2a26..42fb705e 100644
--- a/manifest.json
+++ b/manifest.json
@@ -34,6 +34,7 @@
              "lib/clipboard.js",
              "content_scripts/link_hints.js",
              "content_scripts/vomnibar.js",
+             "content_scripts/scroller.js",
              "content_scripts/vimium_frontend.js"
             ],
       "css": ["vimium.css"],
diff --git a/tests/dom_tests/dom_tests.html b/tests/dom_tests/dom_tests.html
index 5de9e730..898546dc 100644
--- a/tests/dom_tests/dom_tests.html
+++ b/tests/dom_tests/dom_tests.html
@@ -36,6 +36,7 @@
     <script type="text/javascript" src="../../lib/clipboard.js"></script>
     <script type="text/javascript" src="../../content_scripts/link_hints.js"></script>
     <script type="text/javascript" src="../../content_scripts/vomnibar.js"></script>
+    <script type="text/javascript" src="../../content_scripts/scroller.js"></script>
     <script type="text/javascript" src="../../content_scripts/vimium_frontend.js"></script>
 
     <script type="text/javascript" src="../shoulda.js/shoulda.js"></script>