Scroll the body element if last activated node is not visible.

3 files changed, 56 insertions, 56 deletions

diff --git a/lib/utils.js b/lib/utils.js
index ef961833..0c86970e 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -30,4 +30,48 @@ var utils = {
     }
     return document.evaluate(xpath, document.documentElement, namespaceResolver, resultType, null);
   },
+
+  /**
+   * Returns the first visible clientRect of an element if it exists. Otherwise it returns null.
+   */
+  getVisibleClientRect: function(element) {
+    // Note: this call will be expensive if we modify the DOM in between calls.
+    var clientRects = element.getClientRects();
+    var clientRectsLength = clientRects.length;
+
+    for (var i = 0; i < clientRectsLength; i++) {
+      if (clientRects[i].top < 0 || clientRects[i].top >= window.innerHeight - 4 ||
+          clientRects[i].left < 0 || clientRects[i].left  >= window.innerWidth - 4)
+        continue;
+
+      if (clientRects[i].width < 3 || clientRects[i].height < 3)
+        continue;
+
+      // eliminate invisible elements (see test_harnesses/visibility_test.html)
+      var computedStyle = window.getComputedStyle(element, null);
+      if (computedStyle.getPropertyValue('visibility') != 'visible' ||
+          computedStyle.getPropertyValue('display') == 'none')
+        continue;
+
+      return clientRects[i];
+    }
+
+    for (var i = 0; i < clientRectsLength; i++) {
+      // If the link has zero dimensions, it may be wrapping visible
+      // but floated elements. Check for this.
+      if (clientRects[i].width == 0 || clientRects[i].height == 0) {
+        for (var j = 0, childrenCount = element.children.length; j < childrenCount; j++) {
+          var computedStyle = window.getComputedStyle(element.children[j], null);
+          // Ignore child elements which are not floated and not absolutely positioned for parent elements with zero width/height
+          if (computedStyle.getPropertyValue('float') == 'none' && computedStyle.getPropertyValue('position') != 'absolute')
+            continue;
+          var childClientRect = this.getVisibleClientRect(element.children[j]);
+          if (childClientRect === null)
+            continue;
+          return childClientRect;
+        }
+      }
+    };
+    return null;
+  },
 };
diff --git a/linkHints.js b/linkHints.js
index fb59124a..0181436b 100644
--- a/linkHints.js
+++ b/linkHints.js
@@ -120,7 +120,7 @@ var linkHints = {
     // Find all visible clickable elements.
     for (var i = 0, count = resultSet.snapshotLength; i < count; i++) {
       var element = resultSet.snapshotItem(i);
-      var clientRect = this.getVisibleClientRect(element, clientRect);
+      var clientRect = utils.getVisibleClientRect(element, clientRect);
       if (clientRect !== null)
         visibleElements.push({element: element, rect: clientRect});
 
@@ -145,52 +145,6 @@ var linkHints = {
     return visibleElements;
   },
 
-  /**
-   * Returns the first visible clientRect of an element if it exists. Otherwise it returns null.
-   */
-  getVisibleClientRect: function(element) {
-    // Note: this call will be expensive if we modify the DOM in between calls.
-    var clientRects = element.getClientRects();
-    var clientRectsLength = clientRects.length;
-
-    for (var i = 0; i < clientRectsLength; i++) {
-      // Exclude links which have just a few pixels on screen, because the link hints won't show for them
-      // anyway.
-      if (clientRects[i].top < 0 || clientRects[i].top >= window.innerHeight - 4 ||
-          clientRects[i].left < 0 || clientRects[i].left  >= window.innerWidth - 4)
-        continue;
-
-      if (clientRects[i].width < 3 || clientRects[i].height < 3)
-        continue;
-
-      // eliminate invisible elements (see test_harnesses/visibility_test.html)
-      var computedStyle = window.getComputedStyle(element, null);
-      if (computedStyle.getPropertyValue('visibility') != 'visible' ||
-          computedStyle.getPropertyValue('display') == 'none')
-        continue;
-
-      return clientRects[i];
-    }
-
-    for (var i = 0; i < clientRectsLength; i++) {
-      // If the link has zero dimensions, it may be wrapping visible
-      // but floated elements. Check for this.
-      if (clientRects[i].width == 0 || clientRects[i].height == 0) {
-        for (var j = 0, childrenCount = element.children.length; j < childrenCount; j++) {
-          var computedStyle = window.getComputedStyle(element.children[j], null);
-          // Ignore child elements which are not floated and not absolutely positioned for parent elements with zero width/height
-          if (computedStyle.getPropertyValue('float') == 'none' && computedStyle.getPropertyValue('position') != 'absolute')
-            continue;
-          var childClientRect = this.getVisibleClientRect(element.children[j]);
-          if (childClientRect === null)
-            continue;
-          return childClientRect;
-        }
-      }
-    };
-    return null;
-  },
-
   /*
    * Handles shift and esc keys. The other keys are passed to markerMatcher.matchHintsByKey.
    */
diff --git a/vimiumFrontend.js b/vimiumFrontend.js
index 3ae6e6cc..4ab01d2a 100644
--- a/vimiumFrontend.js
+++ b/vimiumFrontend.js
@@ -223,14 +223,15 @@ function onDOMActivate(event) {
  * input elements. This mechanism allows us to decide whether to scroll a div or to scroll the whole document.
  */
 function scrollActivatedElementBy(x, y) {
-  if (!activatedElement) {
-    // if this is called before domReady, just use the window scroll function
-    if (document.body)
-      activatedElement = document.body;
-    else
-      window.scrollBy(x, y);
+  // if this is called before domReady, just use the window scroll function
+  if (!document.body) {
+    window.scrollBy(x, y);
+    return;
   }
 
+  if (!activatedElement || utils.getVisibleClientRect(activatedElement) === null)
+    activatedElement = document.body;
+
   // 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.
@@ -240,7 +241,8 @@ function scrollActivatedElementBy(x, y) {
       var oldScrollTop = element.scrollTop;
       element.scrollTop += y;
       var lastElement = element;
-      element = element.parentElement;
+      // we may have an orphaned element. if so, just scroll the body element.
+      element = element.parentElement || document.body;
     } while(lastElement.scrollTop == oldScrollTop && lastElement.nodeName.toLowerCase() != 'body');
   }
 
@@ -250,7 +252,7 @@ function scrollActivatedElementBy(x, y) {
       var oldScrollLeft = element.scrollLeft;
       element.scrollLeft += x;
       var lastElement = element;
-      element = element.parentElement;
+      element = element.parentElement || document.body;
     } while(lastElement.scrollLeft == oldScrollLeft && lastElement.nodeName.toLowerCase() != 'body');
   }
 
@@ -286,7 +288,7 @@ function focusInput(count) {
     var currentInputBox = results.iterateNext();
     if (!currentInputBox) { break; }
 
-    if (linkHints.getVisibleClientRect(currentInputBox) === null)
+    if (utils.getVisibleClientRect(currentInputBox) === null)
         continue;
 
     lastInputBox = currentInputBox;