Bug 1032724 - Improve spatial navigation, r=blassey

toolkit/modules/SpatialNavigation.jsm

@@ -290,45 +290,28 @@ function _getBestToFocus(nodes, key, currentlyFocused) {
 
     // Initialize best to the first viable value:
     if (!best) {
-      best = nodes[i];
+      bestDist = _spatialDistanceOfCorner(currentlyFocused, nodes[i], key);
-      bestDist = _spatialDistance(best, currentlyFocused);
+      if (bestDist >= 0) {
+        best = nodes[i];
+      }
       continue;
     }
 
     // Of the remaining nodes, pick the one closest to the currently focused
     // node.
-    let curDist = _spatialDistance(nodes[i], currentlyFocused);
+    let curDist = _spatialDistanceOfCorner(currentlyFocused, nodes[i], key);
-    if (curDist > bestDist) {
+    if ((curDist > bestDist) || curDist === -1) {
       continue;
     }
-
+    else if (curDist === bestDist) {
-    bestMid = _getMidpoint(best);
+      let midCurDist = _spatialDistance(currentlyFocused, nodes[i]);
-    switch (key) {
+      let midBestDist = _spatialDistance(currentlyFocused, best);
-      case PrefObserver['keyCodeLeft']:
+      if (midCurDist > midBestDist)
-        if (nodeMid.x > bestMid.x) {
+        continue;
-          best = nodes[i];
-          bestDist = curDist;
-        }
-        break;
-      case PrefObserver['keyCodeRight']:
-        if (nodeMid.x < bestMid.x) {
-          best = nodes[i];
-          bestDist = curDist;
-        }
-        break;
-      case PrefObserver['keyCodeUp']:
-        if (nodeMid.y > bestMid.y) {
-          best = nodes[i];
-          bestDist = curDist;
-        }
-        break;
-      case PrefObserver['keyCodeDown']:
-        if (nodeMid.y < bestMid.y) {
-          best = nodes[i];
-          bestDist = curDist;
-        }
-        break;
     }
+
+    best = nodes[i];
+    bestDist = curDist;
   }
   return best;
 }
@@ -376,23 +359,43 @@ function _getSearchRect(currentlyFocused, key, cssPageRect) {
 
   switch (key) {
     case PrefObserver['keyCodeLeft']:
+      newRect.right = newRect.left;
       newRect.left = cssPageRect.left;
       newRect.width = newRect.right - newRect.left;
-      break;
 
-    case PrefObserver['keyCodeRight']:
+      newRect.bottom = cssPageRect.bottom;
-      newRect.right = cssPageRect.right;
-      newRect.width = newRect.right - newRect.left;
-      break;
-
-    case PrefObserver['keyCodeUp']:
       newRect.top = cssPageRect.top;
       newRect.height = newRect.bottom - newRect.top;
       break;
 
+    case PrefObserver['keyCodeRight']:
+      newRect.left = newRect.right;
+      newRect.right = cssPageRect.right;
+      newRect.width = newRect.right - newRect.left;
+
+      newRect.bottom = cssPageRect.bottom;
+      newRect.top = cssPageRect.top;
+      newRect.height = newRect.bottom - newRect.top;
+      break;
+
+    case PrefObserver['keyCodeUp']:
+      newRect.bottom = newRect.top;
+      newRect.top = cssPageRect.top;
+      newRect.height = newRect.bottom - newRect.top;
+
+      newRect.right = cssPageRect.right;
+      newRect.left = cssPageRect.left;
+      newRect.width = newRect.right - newRect.left;
+      break;
+
     case PrefObserver['keyCodeDown']:
+      newRect.top = newRect.bottom;
       newRect.bottom = cssPageRect.bottom;
       newRect.height = newRect.bottom - newRect.top;
+
+      newRect.right = cssPageRect.right;
+      newRect.left = cssPageRect.left;
+      newRect.width = newRect.right - newRect.left;
       break;
   }
   return newRect;
@@ -407,6 +410,83 @@ function _spatialDistance(a, b) {
                     Math.pow(mida.y - midb.y, 2));
 }
 
+// Get the distance between the corner of two nodes
+function _spatialDistanceOfCorner(from, to, key) {
+  let fromRect = from.getBoundingClientRect();
+  let toRect = to.getBoundingClientRect();
+  let fromMid = _getMidpoint(from);
+  let toMid = _getMidpoint(to);
+  let hDistance = 0;
+  let vDistance = 0;
+
+  switch (key) {
+    case PrefObserver['keyCodeLeft']:
+      // Make sure the "to" node is really at the left side of "from" node by
+      //  1. Check the mid point
+      //  2. The right border of "to" node must be less than the "from" node
+      if ((fromMid.x - toMid.x) < 0 || toRect.right >= fromRect.right)
+        return -1;
+      hDistance = Math.abs(fromRect.left - toRect.right);
+      if (toRect.bottom <= fromRect.top) {
+        vDistance = fromRect.top - toRect.bottom;
+      }
+      else if (fromRect.bottom <= toRect.top) {
+        vDistance = toRect.top - fromRect.bottom;
+      }
+      else {
+        vDistance = 0;
+      }
+      break;
+
+    case PrefObserver['keyCodeRight']:
+      if ((toMid.x - fromMid.x) < 0 || toRect.left <= fromRect.left)
+        return -1;
+      hDistance = Math.abs(toRect.left - fromRect.right);
+      if (toRect.bottom <= fromRect.top) {
+        vDistance = fromRect.top - toRect.bottom;
+      }
+      else if (fromRect.bottom <= toRect.top) {
+        vDistance = toRect.top - fromRect.bottom;
+      }
+      else {
+        vDistance = 0;
+      }
+      break;
+
+    case PrefObserver['keyCodeUp']:
+      if ((fromMid.y - toMid.y) < 0 || toRect.bottom >= fromRect.bottom)
+        return -1;
+      vDistance = Math.abs(fromRect.top - toRect.bottom);
+      if (fromRect.right <= toRect.left) {
+        hDistance = toRect.left - fromRect.right;
+      }
+      else if (toRect.right <= fromRect.left) {
+        hDistance = fromRect.left - toRect.right;
+      }
+      else {
+        hDistance = 0;
+      }
+      break;
+
+    case PrefObserver['keyCodeDown']:
+      if ((toMid.y - fromMid.y) < 0 || toRect.top <= fromRect.top)
+        return -1;
+      vDistance = Math.abs(toRect.top - fromRect.bottom);
+      if (fromRect.right <= toRect.left) {
+        hDistance = toRect.left - fromRect.right;
+      }
+      else if (toRect.right <= fromRect.left) {
+        hDistance = fromRect.left - toRect.right;
+      }
+      else {
+        hDistance = 0;
+      }
+      break;
+  }
+  return Math.round(Math.pow(hDistance, 2) +
+                    Math.pow(vDistance, 2));
+}
+
 // Snav preference observer
 var PrefObserver = {
   register: function() {
@@ -524,4 +604,4 @@ var PrefObserver = {
   }
 };
 
-PrefObserver.register();
+PrefObserver.register();

toolkit/modules/tests/mochitest/test_spatial_navigation.html

@@ -39,22 +39,22 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=698437
                                              " not become the active element?");
 
     synthesizeKey("VK_UP", { });
-    is(top.id, document.activeElement.id,
+    is(document.activeElement.id, top.id,
        "Spatial navigation up key is not handled correctly.");
 
     center.focus();
     synthesizeKey("VK_DOWN", { });
-    is(bottom.id, document.activeElement.id,
+    is(document.activeElement.id, bottom.id,
        "Spatial navigation down key is not handled correctly.");
 
     center.focus();
     synthesizeKey("VK_RIGHT", { });
-    is(right.id, document.activeElement.id,
+    is(document.activeElement.id, right.id,
        "Spatial navigation right key is not handled correctly.");
 
     center.focus();
     synthesizeKey("VK_LEFT", { });
-    is(left.id, document.activeElement.id,
+    is(document.activeElement.id, left.id,
        "Spatial navigation left key is not handled correctly.");
 
     SimpleTest.finish();
@@ -66,11 +66,9 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=698437
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=698437">Mozilla Bug 698437</a>
 <p id="display"></p>
 <div id="content">
-  <p> This is a <a id="top" href="#">really</a> long sentence </p>
+  <button id="lefttop">1</button><button id="top">2</button><button id="righttop">3</button><br>
-  <p> <a id="left" href="#">This</a> is a
+  <button id="left">4</button><button id="center">5</button><button id="right">6</button><br>
-      <a id="center" href="#">really</a> long
+  <button id="leftbottom">7</button><button id="bottom">8</button><button id="rightbottom">9</button><br>
-      <a id="right" href="#">sentence</a> </p>
-  <p> This is a <a id="bottom" href="#">really</a> long sentence </p>
 </div>
 <pre id="test">
 </pre>