Bug 828895 - keyboard navigation in the Library View is bogus.
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 10 Jan 2013 21:48:03 +0100
changeset 118461 7e4917d358c50cc8e8ec069887c1daeb8c9b9f4a
parent 118460 945ed43286282df09246bc03c1b1183d95b3bcf3
child 118462 b99c87fb77d33c6bb40262680a7b86aabf71cfd1
push id24166
push userMs2ger@gmail.com
push dateFri, 11 Jan 2013 13:57:41 +0000
treeherdermozilla-central@63c4b0f66a0c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs828895
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 828895 - keyboard navigation in the Library View is bogus. r=Mano
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) {