From c972978b43b943a1ad8709992d080bedbbe12ae2 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 17 Dec 2014 16:44:47 +0000 Subject: Add comment re. Math.sign(). --- content_scripts/scroller.coffee | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'content_scripts/scroller.coffee') diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee index 09470158..2f69fc7d 100644 --- a/content_scripts/scroller.coffee +++ b/content_scripts/scroller.coffee @@ -5,6 +5,10 @@ activatedElement = null # 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 +# only introduced in Chrome 38. This caused problems in R1.48 for users with old Chrome installations. We +# can replace this with Math.sign() at some point. getSign = (val) -> if not val 0 -- cgit v1.2.3 From 0506ea78ebcd003b47db4f5587e07251f7c8682b Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 19 Dec 2014 07:10:02 +0000 Subject: Initialize scroller to first scrollable element. See #1358. --- content_scripts/scroller.coffee | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'content_scripts/scroller.coffee') diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee index 2f69fc7d..48b99cff 100644 --- a/content_scripts/scroller.coffee +++ b/content_scripts/scroller.coffee @@ -84,6 +84,16 @@ findScrollableElement = (element, direction, amount, factor) -> element = element.parentElement || document.body element +# On some pages, document.body is not scrollable. Here, we search the document for the first visible element +# which does scroll vertically. This is used to initialize activatedElement. See #1358. +firstScrollableElement = (element=document.body) -> + if doesScroll element, "y", 1, 1 + element + else + for child in element.children + return ele if DomUtils.getVisibleClientRect(child) and ele = firstScrollableElement child + null + checkVisibility = (element) -> # If the activated element has been scrolled completely offscreen, then subsequent changes in its scroll # position will not provide any more visual feedback to the user. Therefore, we deactivate it so that @@ -206,7 +216,7 @@ Scroller = window.scrollBy(0, amount) return - activatedElement ||= document.body + activatedElement ||= firstScrollableElement() || document.body return unless activatedElement # Avoid the expensive scroll calculation if it will not be used. This reduces costs during smooth, @@ -218,7 +228,7 @@ Scroller = scrollTo: (direction, pos) -> return unless document.body or activatedElement - activatedElement ||= document.body + activatedElement ||= firstScrollableElement() || document.body element = findScrollableElement activatedElement, direction, pos, 1 amount = getDimension(element,direction,pos) - element[scrollProperties[direction].axisName] -- cgit v1.2.3 From ac2e1077107d72ef82e1424634aa86945696e6b4 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 19 Dec 2014 08:35:03 +0000 Subject: Initialize scroller to *largest* visible scrollable element. --- content_scripts/scroller.coffee | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'content_scripts/scroller.coffee') diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee index 48b99cff..dec817a1 100644 --- a/content_scripts/scroller.coffee +++ b/content_scripts/scroller.coffee @@ -84,14 +84,17 @@ findScrollableElement = (element, direction, amount, factor) -> element = element.parentElement || document.body element -# On some pages, document.body is not scrollable. Here, we search the document for the first visible element -# which does scroll vertically. This is used to initialize activatedElement. See #1358. +# On some pages, document.body 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) -> if doesScroll element, "y", 1, 1 element else - for child in element.children - return ele if DomUtils.getVisibleClientRect(child) and ele = firstScrollableElement child + children = ({element: child, rect: DomUtils.getVisibleClientRect(child)} for child in element.children) + children = children.filter (child) -> child.rect # Filter out non-visible elements. + children.map (child) -> child.area = child.rect.width * child.rect.height + for child in children.sort((a,b) -> b.area - a.area) # Largest to smallest by visible area. + return ele if ele = firstScrollableElement child.element null checkVisibility = (element) -> -- cgit v1.2.3 From 24b968943acddd224dd795c1e26425d3b75520e2 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 19 Dec 2014 08:49:01 +0000 Subject: Delay initialization of activeElement. We could incorrectly initialize activeElement to document.body if the scroller is called too early; so delay initialization. It's safe to leave activeElement as null. --- content_scripts/scroller.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'content_scripts/scroller.coffee') diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee index dec817a1..3692a002 100644 --- a/content_scripts/scroller.coffee +++ b/content_scripts/scroller.coffee @@ -219,7 +219,7 @@ Scroller = window.scrollBy(0, amount) return - activatedElement ||= firstScrollableElement() || document.body + activatedElement ||= firstScrollableElement() return unless activatedElement # Avoid the expensive scroll calculation if it will not be used. This reduces costs during smooth, @@ -230,8 +230,8 @@ Scroller = CoreScroller.scroll element, direction, elementAmount scrollTo: (direction, pos) -> - return unless document.body or activatedElement - activatedElement ||= firstScrollableElement() || document.body + activatedElement ||= firstScrollableElement() + return unless activatedElement element = findScrollableElement activatedElement, direction, pos, 1 amount = getDimension(element,direction,pos) - element[scrollProperties[direction].axisName] -- cgit v1.2.3 From 5f4ae0f11340c0c7385d8a91228941128a9731da Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 19 Dec 2014 11:25:51 +0000 Subject: Also test negative direction when initializing scroller. The right scrollable element to choose may be scrolled to the bottom, so we won't find it if we only test scrolling down. We need to test scrolling up as well. --- content_scripts/scroller.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'content_scripts/scroller.coffee') diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee index 3692a002..fdfb7ddc 100644 --- a/content_scripts/scroller.coffee +++ b/content_scripts/scroller.coffee @@ -67,7 +67,7 @@ shouldScroll = (element, direction) -> # Instead, we scroll the element by 1 or -1 and see if it moved (then put it back). :factor is the factor by # which :scrollBy and :scrollTo will later scale the scroll amount. :factor can be negative, so we need it # here in order to decide whether we should test a forward scroll or a backward scroll. -# Bug verified in Chrome 38.0.2125.104. +# Bug last verified in Chrome 38.0.2125.104. doesScroll = (element, direction, amount, factor) -> # amount is treated as a relative amount, which is correct for relative scrolls. For absolute scrolls (only # gg, G, and friends), amount can be either a string ("max" or "viewSize") or zero. In the former case, @@ -87,7 +87,7 @@ findScrollableElement = (element, direction, amount, factor) -> # On some pages, document.body 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) -> - if doesScroll element, "y", 1, 1 + if doesScroll(element, "y", 1, 1) or doesScroll(element, "y", -1, 1) element else children = ({element: child, rect: DomUtils.getVisibleClientRect(child)} for child in element.children) -- cgit v1.2.3 From 6c1bbf0ab5781951364464c5fa68ad22f74c9fee Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Mon, 29 Dec 2014 07:15:44 +0000 Subject: Scroller; check document.body exists. --- content_scripts/scroller.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'content_scripts/scroller.coffee') diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee index fdfb7ddc..889dc042 100644 --- a/content_scripts/scroller.coffee +++ b/content_scripts/scroller.coffee @@ -219,7 +219,7 @@ Scroller = window.scrollBy(0, amount) return - activatedElement ||= firstScrollableElement() + activatedElement ||= document.body and firstScrollableElement() return unless activatedElement # Avoid the expensive scroll calculation if it will not be used. This reduces costs during smooth, @@ -230,7 +230,7 @@ Scroller = CoreScroller.scroll element, direction, elementAmount scrollTo: (direction, pos) -> - activatedElement ||= firstScrollableElement() + activatedElement ||= document.body and firstScrollableElement() return unless activatedElement element = findScrollableElement activatedElement, direction, pos, 1 -- cgit v1.2.3