From 9b5e26b9009c1ff6b5c55ebd4d0e6c3915df0261 Mon Sep 17 00:00:00 2001 From: Darryl Pogue Date: Wed, 15 Jun 2016 22:10:19 -0700 Subject: Use document.scrollingElement for scrolling This fixes Vimium with the 'Experimental Web Platform Features' flag enabled, and ensures compatibility across quirks mode scrolling (via document.body) and standards-compliant scrolling (via document.documentElement). --- content_scripts/scroller.coffee | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee index 56b9a6c6..564c84c8 100644 --- a/content_scripts/scroller.coffee +++ b/content_scripts/scroller.coffee @@ -4,6 +4,9 @@ # activatedElement = null +if !('scrollingElement' of document) + Object.defineProperty(document, 'scrollingElement', { get: () -> 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 # (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/sign) Math.sign() was @@ -37,7 +40,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.body + if name is 'viewSize' and el is document.scrollingElement # TODO(smblott) Should we not be returning the width/height of element, here? if direction is 'x' then window.innerWidth else window.innerHeight else @@ -82,13 +85,13 @@ 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.body and not isScrollableElement element, direction, amount, factor - element = DomUtils.getContainingElement(element) ? document.body + while element != document.scrollingElement and not isScrollableElement element, direction, amount, factor + element = DomUtils.getContainingElement(element) ? document.scrollingElement element -# On some pages, document.body is not scrollable. Here, we search the document for the largest visible +# 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.body) -> +firstScrollableElement = (element=document.scrollingElement) -> if doesScroll(element, "y", 1, 1) or doesScroll(element, "y", -1, 1) element else @@ -234,14 +237,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.body and amount instanceof Number) + if (!document.scrollingElement and amount instanceof Number) if (direction == "x") window.scrollBy(amount, 0) else window.scrollBy(0, amount) return - activatedElement ||= (document.body and firstScrollableElement()) or document.body + activatedElement ||= (document.scrollingElement and firstScrollableElement()) or document.scrollingElement return unless activatedElement # Avoid the expensive scroll calculation if it will not be used. This reduces costs during smooth, @@ -252,7 +255,7 @@ Scroller = CoreScroller.scroll element, direction, elementAmount, continuous scrollTo: (direction, pos) -> - activatedElement ||= (document.body and firstScrollableElement()) or document.body + activatedElement ||= (document.scrollingElement and firstScrollableElement()) or document.scrollingElement return unless activatedElement element = findScrollableElement activatedElement, direction, pos, 1 @@ -261,13 +264,13 @@ Scroller = # Is element scrollable and not the activated element? isScrollableElement: (element) -> - activatedElement ||= (document.body and firstScrollableElement()) or document.body + activatedElement ||= (document.scrollingElement and firstScrollableElement()) or document.scrollingElement 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.body and firstScrollableElement() + activatedElement ||= document.scrollingElement and firstScrollableElement() rect = element. getClientRects()?[0] if rect? # Scroll y axis. -- cgit v1.2.3 From 07bc4fbe820ff17b5a6b559a2ebcceb7d52700a2 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 24 Sep 2016 07:17:44 +0100 Subject: 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. --- CREDITS | 1 + content_scripts/scroller.coffee | 30 +++++++++++++++++------------- 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 (github: PrestanceDesign) Dahan Gong (github: gdh1995) Scott Pinkelman (github: sco-tt) + Darryl Pogue (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. -- cgit v1.2.3