Bug 543065, only scroll one ancestor of menus, SCROLL_FIRST_ANCESTOR_ONLY flag should stop at first scrollable container, not just first parent, r=roc

layout/base/nsPresShell.cpp

@@ -4137,6 +4137,11 @@ PresShell::ScrollFrameRectIntoView(nsIFrame*     aFrame,
         didScroll = PR_TRUE;
       }
 
+      // only scroll one container when this flag is set
+      if (aFlags & SCROLL_FIRST_ANCESTOR_ONLY) {
+        break;
+      }
+
       nsRect scrollPort = sf->GetScrollPortRect();
       if (rect.XMost() < scrollPort.x ||
           rect.x > scrollPort.XMost() ||
@@ -4155,7 +4160,7 @@ PresShell::ScrollFrameRectIntoView(nsIFrame*     aFrame,
     }
     rect += container->GetPosition();
     container = container->GetParent();
-  } while (container && !(aFlags & SCROLL_FIRST_ANCESTOR_ONLY));
+  } while (container);
 
   return didScroll;
 }

layout/xul/base/src/nsMenuPopupFrame.cpp

@@ -1253,7 +1253,8 @@ void nsMenuPopupFrame::EnsureMenuItemIsVisible(nsMenuFrame* aMenuItem)
                               nsRect(nsPoint(0,0), aMenuItem->GetRect().Size()),
                               NS_PRESSHELL_SCROLL_ANYWHERE,
                               NS_PRESSHELL_SCROLL_ANYWHERE,
-                              nsIPresShell::SCROLL_OVERFLOW_HIDDEN);
+                              nsIPresShell::SCROLL_OVERFLOW_HIDDEN |
+                              nsIPresShell::SCROLL_FIRST_ANCESTOR_ONLY);
   }
 }
 

toolkit/content/tests/widgets/test_menulist_keynav.xul

@@ -3,18 +3,15 @@
 <?xml-stylesheet href="/tests/SimpleTest/test.css" type="text/css"?>
 
 <window title="Menulist Key Navigation Tests"
-  onload="setTimeout(runTests, 0);"
-  xmlns:html="http://www.w3.org/1999/xhtml"
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
 
-  <title>Menulist Key Navigation Tests</title>
   <script type="application/javascript" src="/MochiKit/packed.js"></script>
   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>      
   <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>      
 
 <button id="button1" label="One"/>
 <menulist id="list">
-  <menupopup id="popup" onpopupshowing="return false;">
+  <menupopup id="popup" onpopupshowing="return gShowPopup;">
     <menuitem id="i1" label="One"/>
     <menuitem id="i2" label="Two"/>
     <menuitem id="i2b" disabled="true" label="Two and a Half"/>
@@ -29,6 +26,8 @@
 
 SimpleTest.waitForExplicitFinish();
 
+var gShowPopup = false;
+
 var iswin = (navigator.platform.indexOf("Win") == 0);
 
 function runTests()
@@ -70,15 +69,42 @@ function pressedAgain()
 
 function differentPressed()
 {
-  keyCheck($("list"), "O", 1, "different letter pressed");
+  var list = $("list");
+  keyCheck(list, "O", 1, "different letter pressed");
 
   if (navigator.platform.indexOf("Mac") == -1) {
     $("button1").focus();
-    synthesizeKeyExpectEvent("VK_TAB", { }, $("list"), "focus", "focus to menulist");
+    synthesizeKeyExpectEvent("VK_TAB", { }, list, "focus", "focus to menulist");
     synthesizeKeyExpectEvent("VK_TAB", { }, $("button2"), "focus", "focus to button");
     is(document.activeElement, $("button2"), "tab from menulist focused button");
   }
 
+  // now make sure that using a key scrolls the menu correctly
+  gShowPopup = true;
+
+  for (let i = 0; i < 15; i++) {
+    list.appendItem("Item" + i, "item" + i);
+  }
+  list.menupopup.maxHeight = 100;
+  list.open = true;
+
+  var rowdiff = list.getItemAtIndex(1).getBoundingClientRect().top -
+                list.getItemAtIndex(0).getBoundingClientRect().top;
+
+  var item = list.getItemAtIndex(10);
+  var originalPosition = item.getBoundingClientRect().top;
+
+  list.menuBoxObject.activeChild = item;
+  ok(item.getBoundingClientRect().top < originalPosition,
+    "position of item 1: " + item.getBoundingClientRect().top + " -> " + originalPosition);
+
+  originalPosition = item.getBoundingClientRect().top;
+
+  synthesizeKey("VK_DOWN", { });
+  is(item.getBoundingClientRect().top, originalPosition - rowdiff, "position of item 10");
+
+  list.open = false;
+
   SimpleTest.finish();
 }
 
@@ -89,6 +115,8 @@ function keyCheck(list, key, index, testname)
   is(list.selectedItem, item, testname + " selectedItem");
 }
 
+SimpleTest.waitForFocus(runTests);
+
 ]]>
 </script>