Tweak #2168 (scrolling via scrollingElement).

Instead of setting a property of document for `scrollingElement` (if it
is not defined), just use a function and make the decision dynamically
instead.

2 files changed, 18 insertions, 13 deletions

diff --git a/CREDITS b/CREDITS
index 62e79698..1caab17d 100644
--- a/CREDITS
+++ b/CREDITS
@@ -46,5 +46,6 @@ Contributors:
   Michael Salihi <admin@prestance-informatique.fr> (github: PrestanceDesign)
   Dahan Gong <gdh1995@qq.com> (github: gdh1995)
   Scott Pinkelman <scott@scottpinkelman.com> (github: sco-tt)
+  Darryl Pogue <darryl@dpogue.ca> (github: dpogue)
 
 Feel free to add real names in addition to GitHub usernames.
diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee
index 564c84c8..3a1b3772 100644
--- a/content_scripts/scroller.coffee
+++ b/content_scripts/scroller.coffee
@@ -4,8 +4,11 @@
 #
 activatedElement = null
 
-if !('scrollingElement' of document)
-  Object.defineProperty(document, 'scrollingElement', { get: () -> document.body })
+# Previously, the main scrolling element was document.body.  If the "experimental web platform features" flag
+# is enabled, then we need to use document.scrollingElement instead.  There's an explanation in #2168:
+# https://github.com/philc/vimium/pull/2168#issuecomment-236488091
+
+getScrollingElement = -> document.scrollingElement ? document.body
 
 # Return 0, -1 or 1: the sign of the argument.
 # NOTE(smblott; 2014/12/17) We would like to use Math.sign().  However, according to this site
@@ -40,7 +43,7 @@ getDimension = (el, direction, amount) ->
     name = amount
     # 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.scrollingElement
+    if name is 'viewSize' and el is getScrollingElement()
       # TODO(smblott) Should we not be returning the width/height of element, here?
       if direction is 'x' then window.innerWidth else window.innerHeight
     else
@@ -85,13 +88,14 @@ isScrollableElement = (element, direction = "y", amount = 1, factor = 1) ->
 
 # From element and its parents, find the first which we should scroll and which does scroll.
 findScrollableElement = (element, direction, amount, factor) ->
-  while element != document.scrollingElement and not isScrollableElement element, direction, amount, factor
-    element = DomUtils.getContainingElement(element) ? document.scrollingElement
+  while element != getScrollingElement() and not isScrollableElement element, direction, amount, factor
+    element = DomUtils.getContainingElement(element) ? getScrollingElement()
   element
 
-# On some pages, document.scrollingElement is not scrollable.  Here, we search the document for the largest visible
-# element which does scroll vertically. This is used to initialize activatedElement. See #1358.
-firstScrollableElement = (element=document.scrollingElement) ->
+# On some pages, the scrolling element is not actually scrollable.  Here, we search the document for the
+# largest visible element which does scroll vertically. This is used to initialize activatedElement. See
+# #1358.
+firstScrollableElement = (element=getScrollingElement()) ->
   if doesScroll(element, "y", 1, 1) or doesScroll(element, "y", -1, 1)
     element
   else
@@ -237,14 +241,14 @@ Scroller =
   # :factor is needed because :amount can take on string values, which scrollBy converts to element dimensions.
   scrollBy: (direction, amount, factor = 1, continuous = true) ->
     # if this is called before domReady, just use the window scroll function
-    if (!document.scrollingElement and amount instanceof Number)
+    if (!getScrollingElement() and amount instanceof Number)
       if (direction == "x")
         window.scrollBy(amount, 0)
       else
         window.scrollBy(0, amount)
       return
 
-    activatedElement ||= (document.scrollingElement and firstScrollableElement()) or document.scrollingElement
+    activatedElement ||= (getScrollingElement() and firstScrollableElement()) or getScrollingElement()
     return unless activatedElement
 
     # Avoid the expensive scroll calculation if it will not be used.  This reduces costs during smooth,
@@ -255,7 +259,7 @@ Scroller =
       CoreScroller.scroll element, direction, elementAmount, continuous
 
   scrollTo: (direction, pos) ->
-    activatedElement ||= (document.scrollingElement and firstScrollableElement()) or document.scrollingElement
+    activatedElement ||= (getScrollingElement() and firstScrollableElement()) or getScrollingElement()
     return unless activatedElement
 
     element = findScrollableElement activatedElement, direction, pos, 1
@@ -264,13 +268,13 @@ Scroller =
 
   # Is element scrollable and not the activated element?
   isScrollableElement: (element) ->
-    activatedElement ||= (document.scrollingElement and firstScrollableElement()) or document.scrollingElement
+    activatedElement ||= (getScrollingElement() and firstScrollableElement()) or getScrollingElement()
     element != activatedElement and isScrollableElement element
 
   # Scroll the top, bottom, left and right of element into view.  The is used by visual mode to ensure the
   # focus remains visible.
   scrollIntoView: (element) ->
-    activatedElement ||= document.scrollingElement and firstScrollableElement()
+    activatedElement ||= getScrollingElement() and firstScrollableElement()
     rect = element. getClientRects()?[0]
     if rect?
       # Scroll y axis.