Bug 828895 - keyboard navigation in the Library View is bogus.
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 10 Jan 2013 21:48:03 +0100
changeset 127066 a19ec2aee33f0127b9923985fb1a75e44092188f
parent 127065 249c11a3f31842cdf23fcda32aa24ab78a69b059
child 127067 43e7be276fb7e8b4f2ab932ac4332222e7317b24
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs828895
milestone20.0a2
Bug 828895 - keyboard navigation in the Library View is bogus. r=Mano a=gavin
browser/components/downloads/content/allDownloadsViewOverlay.js
--- a/browser/components/downloads/content/allDownloadsViewOverlay.js
+++ b/browser/components/downloads/content/allDownloadsViewOverlay.js
@@ -948,37 +948,48 @@ DownloadsPlacesView.prototype = {
     if (!this.active || this._ensureVisibleTimer || !this._richlistbox.firstChild)
       return;
 
     this._ensureVisibleTimer = setTimeout(function() {
       delete this._ensureVisibleTimer;
       if (!this._richlistbox.firstChild)
         return;
 
-      let rlRect = this._richlistbox.getBoundingClientRect();
-      let fcRect = this._richlistbox.firstChild.getBoundingClientRect();
-      // For simplicity assume border and padding are the same across all sides.
-      // This works as far as there isn't an horizontal scrollbar since fcRect
-      // is relative to the scrolled area.
-      let offset = (fcRect.left - rlRect.left) + 1;
-
-      let firstVisible = document.elementFromPoint(fcRect.left, rlRect.top + offset);
-      if (!firstVisible || firstVisible.localName != "richlistitem")
-        throw new Error("_ensureVisibleElementsAreActive invoked on the wrong view");
+      let rlbRect = this._richlistbox.getBoundingClientRect();
+      let winUtils = window.QueryInterface(Ci.nsIInterfaceRequestor)
+                           .getInterface(Ci.nsIDOMWindowUtils);
+      let nodes = winUtils.nodesFromRect(rlbRect.left, rlbRect.top,
+                                         0, rlbRect.width, rlbRect.height, 0,
+                                         true, false);
+      // nodesFromRect returns nodes in z-index order, and for the same z-index
+      // sorts them in inverted DOM order, thus starting from the one that would
+      // be on top.
+      let firstVisibleNode, lastVisibleNode;
+      for (let node of nodes) {
+        if (node.localName === "richlistitem" && node._shell) {
+          node._shell.ensureActive();
+          // The first visible node is the last match.
+          firstVisibleNode = node;
+          // While the last visible node is the first match.
+          if (!lastVisibleNode)
+            lastVisibleNode = node;
+        }
+      }
 
-      let lastVisible = document.elementFromPoint(fcRect.left, rlRect.bottom - offset);
-      // If the last visible child found is not a richlistitem, then there are
-      // less items than the available space, thus just proceed to the last child.
-      if (!lastVisible || lastVisible.localName != "richlistitem")
-        lastVisible = this._richlistbox.lastChild;
+      // Also activate the first invisible nodes in both boundaries (that is,
+      // above and below the visible area) to ensure proper keyboard navigation
+      // in both directions.
+      let nodeBelowVisibleArea = lastVisibleNode && lastVisibleNode.nextSibling;
+      if (nodeBelowVisibleArea && nodeBelowVisibleArea._shell)
+        nodeBelowVisibleArea._shell.ensureActive();
 
-      for (let elt = firstVisible; elt != lastVisible.nextSibling; elt = elt.nextSibling) {
-        if (elt._shell)
-          elt._shell.ensureActive();
-      }
+      let nodeABoveVisibleArea =
+        firstVisibleNode && firstVisibleNode.previousSibling;
+      if (nodeABoveVisibleArea && nodeABoveVisibleArea._shell)
+        nodeABoveVisibleArea._shell.ensureActive();
     }.bind(this), 10);
   },
 
   _place: "",
   get place() this._place,
   set place(val) {
     // Don't reload everything if we don't have to.
     if (this._place == val) {