Bug 490178 - Need a way to set the direction on a richlistbox. r=enn
☠☠ backed out by d62b429af164 ☠ ☠
authorNochum Sossonko <highmind63@gmail.com>
Fri, 14 Aug 2009 23:15:59 +0200
changeset 31570 8f77197a8f99c3a3fdf988033966e0139b604bca
parent 31569 11bd05bebf858c587a990437f07c582aa8988b2d
child 31571 aff827e779e6e94e0a44deb465c79c02996e02fc
child 31575 d62b429af1648e72209b2dce223b48bc6e2c6f7a
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersenn
bugs490178
milestone1.9.3a1pre
Bug 490178 - Need a way to set the direction on a richlistbox. r=enn
toolkit/content/tests/widgets/Makefile.in
toolkit/content/tests/widgets/test_richlist_direction.xul
toolkit/content/widgets/listbox.xml
toolkit/content/widgets/richlistbox.xml
--- a/toolkit/content/tests/widgets/Makefile.in
+++ b/toolkit/content/tests/widgets/Makefile.in
@@ -107,16 +107,17 @@ include $(topsrcdir)/config/rules.mk
 		test_tabindex.xul \
  		test_mousescroll.xul \
 		test_scrollbar.xul \
 		test_sorttemplate.xul \
 		test_contextmenu_list.xul \
 		test_videocontrols.html \
 		test_videocontrols_video_direction.html \
 		test_videocontrols_audio_direction.html \
+		test_richlist_direction.xul \
 		videocontrols_direction-1-ref.html \
 		videocontrols_direction-1a.html \
 		videocontrols_direction-1b.html \
 		videocontrols_direction-1c.html \
 		videocontrols_direction-1d.html \
 		videocontrols_direction-1e.html \
 		videocontrols_direction-2-ref.html \
 		videocontrols_direction-2a.html \
new file mode 100644
--- /dev/null
+++ b/toolkit/content/tests/widgets/test_richlist_direction.xul
@@ -0,0 +1,130 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
+<!--
+  XUL Widget Test for listbox direction
+  -->
+<window title="Listbox direction test"
+        onload="test_richlistbox()"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <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>      
+
+  <richlistbox seltype="multiple" id="richlistbox" flex="1" maxheight="80" height="80" maxwidth="150"/>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml" style="height: 300px; overflow: auto;"/>
+
+<script type="application/javascript">
+<![CDATA[
+
+SimpleTest.waitForExplicitFinish();
+
+var richListBox = document.getElementById("richlistbox");
+
+function getScrollIndexAmount(aDirection) {
+  return (4 * aDirection + richListBox.currentIndex);
+}
+
+function test_richlistbox()
+{
+  var height = richListBox.scrollBoxObject.height;
+  var item;
+  do {
+    item = richListBox.appendItem("Test", "");
+    item.height = item.maxHeight = Math.floor(height / 4);
+  } while (item.getBoundingClientRect().bottom < (height * 2))
+  richListBox.appendItem("Test", "");
+  richListBox.firstChild.nextSibling.id = "list-box-first";
+  richListBox.lastChild.previousSibling.id = "list-box-last";
+
+  // direction = "reverse", the values here are backwards due to the fact that
+  // richlistboxes respond differently when a user initiates a selection
+  richListBox.dir = "reverse";
+  var count = richListBox.itemCount;
+  richListBox.selectedIndex = count - 1;
+  sendKey("DOWN", richListBox);
+  is(richListBox.currentIndex, count - 2, "Selection should move to the next item");
+  sendKey("UP", richListBox);
+  is(richListBox.currentIndex, count - 1, "Selection should move to the previous item");
+  sendKey("END", richListBox);
+  is(richListBox.currentIndex, 0, "Selection should move to the last item");
+  sendKey("HOME", richListBox);
+  is(richListBox.currentIndex, count - 1, "Selection should move to the first item");
+  var currentIndex = richListBox.currentIndex;
+  var index = getScrollIndexAmount(-1);
+  sendKey("PAGE_DOWN", richListBox);
+  is(richListBox.currentIndex, index, "Selection should move to one page down");
+  ok(richListBox.currentIndex < currentIndex, "Selection should move downwards");
+  sendKey("END", richListBox);
+  currentIndex = richListBox.currentIndex;
+  index = getScrollIndexAmount(1);
+  sendKey("PAGE_UP", richListBox);
+  is(richListBox.currentIndex, index, "Selection should move to one page up");
+  ok(richListBox.currentIndex > currentIndex, "Selection should move upwards");
+  richListBox.selectedItem = richListBox.lastChild;
+  richListBox.focus();
+  synthesizeKey("VK_DOWN", {shiftKey: true, type: "keypress"}, window);
+  let items = [richListBox.selectedItems[0],
+               richListBox.selectedItems[1]];
+  is(items[0], richListBox.lastChild, "The last element should still be selected");
+  is(items[1], richListBox.lastChild.previousSibling, "Both elements should now be selected");
+  richListBox.clearSelection();
+  richListBox.selectedItem = richListBox.lastChild;
+  sendMouseEvent({type: "click", shiftKey: true, clickCount: 1},
+                 "list-box-last",
+                 window);
+  items = [richListBox.selectedItems[0],
+           richListBox.selectedItems[1]];
+  is(items[0], richListBox.lastChild, "The last element should still be selected");
+  is(items[1], richListBox.lastChild.previousSibling, "Both elements should now be selected");
+
+  // direction = "normal"
+  richListBox.dir = "normal";
+  richListBox.selectedIndex = 0;
+  sendKey("DOWN", richListBox);
+  is(richListBox.currentIndex, 1, "Selection should move to the next item");
+  sendKey("UP", richListBox);
+  is(richListBox.currentIndex, 0, "Selection should move to the previous item");
+  sendKey("END", richListBox);
+  is(richListBox.currentIndex, count - 1, "Selection should move to the last item");
+  sendKey("HOME", richListBox);
+  is(richListBox.currentIndex, 0, "Selection should move to the first item");
+  var currentIndex = richListBox.currentIndex;
+  var index = richListBox.scrollOnePage(1);
+  sendKey("PAGE_DOWN", richListBox);
+  is(richListBox.currentIndex, index, "Selection should move to one page down");
+  ok(richListBox.currentIndex > currentIndex, "Selection should move downwards");
+  sendKey("END", richListBox);
+  currentIndex = richListBox.currentIndex;
+  index = richListBox.scrollOnePage(-1) + richListBox.currentIndex;
+  sendKey("PAGE_UP", richListBox);
+  is(richListBox.currentIndex, index, "Selection should move to one page up");
+  ok(richListBox.currentIndex < currentIndex, "Selection should move upwards");
+  richListBox.selectedItem = richListBox.firstChild;
+  richListBox.focus();
+  synthesizeKey("VK_DOWN", {shiftKey: true, type: "keypress"}, window);
+  items = [richListBox.selectedItems[0],
+           richListBox.selectedItems[1]];
+  is(items[0], richListBox.firstChild, "The last element should still be selected");
+  is(items[1], richListBox.firstChild.nextSibling, "Both elements should now be selected");
+  richListBox.clearSelection();
+  richListBox.selectedItem = richListBox.firstChild;
+  sendMouseEvent({type: "click", shiftKey: true, clickCount: 1},
+                 "list-box-first",
+                 window);
+  items = [richListBox.selectedItems[0],
+           richListBox.selectedItems[1]];
+  is(items[0], richListBox.firstChild, "The last element should still be selected");
+  is(items[1], richListBox.firstChild.nextSibling, "Both elements should now be selected");
+  SimpleTest.finish();
+}
+
+]]>
+</script>
+
+</window>
--- a/toolkit/content/widgets/listbox.xml
+++ b/toolkit/content/widgets/listbox.xml
@@ -542,18 +542,20 @@
 
       <method name="_moveByOffsetFromUserEvent">
         <parameter name="aOffset"/>
         <parameter name="aEvent"/>
         <body>
         <![CDATA[
           if (!aEvent.getPreventDefault()) {
             this._userSelecting = true;
+            this._mayReverse = true;
             this.moveByOffset(aOffset, !aEvent.ctrlKey, aEvent.shiftKey);
             this._userSelecting = false;
+            this._mayReverse = false;
             aEvent.preventDefault();
           }
         ]]>
         </body>
       </method>
 
       <method name="_canUserSelect">
         <parameter name="aItem"/>
@@ -570,40 +572,57 @@
         <body>
           aMe._fireOnSelect();
           aMe._selectTimeout = null;
         </body>
       </method>
 
       <field name="_suppressOnSelect">false</field>
       <field name="_userSelecting">false</field>
+      <field name="_mayReverse">false</field>
       <field name="_selectTimeout">null</field>
       <field name="_currentItem">null</field>
       <field name="_selectionStart">null</field>
     </implementation>
 
     <handlers>
       <handler event="keypress" keycode="VK_UP" modifiers="control shift any"
                action="this._moveByOffsetFromUserEvent(-1, event);"
                group="system"/>
       <handler event="keypress" keycode="VK_DOWN" modifiers="control shift any"
                action="this._moveByOffsetFromUserEvent(1, event);"
                group="system"/>
       <handler event="keypress" keycode="VK_HOME" modifiers="control shift any"
-               action="this._moveByOffsetFromUserEvent(-this.currentIndex, event);"
-               group="system"/>
+               group="system">
+        <![CDATA[
+          this._mayReverse = true;
+          this._moveByOffsetFromUserEvent(-this.currentIndex, event);
+        ]]>
+      </handler>
       <handler event="keypress" keycode="VK_END" modifiers="control shift any"
-               action="this._moveByOffsetFromUserEvent(this.getRowCount() - this.currentIndex - 1, event);"
-               group="system"/>
+               group="system">
+        <![CDATA[
+          this._mayReverse = true;
+          this._moveByOffsetFromUserEvent(this.getRowCount() - this.currentIndex - 1, event);
+        ]]>
+      </handler>
       <handler event="keypress" keycode="VK_PAGE_UP" modifiers="control shift any"
-               action="this._moveByOffsetFromUserEvent(this.scrollOnePage(-1), event);"
-               group="system"/>
+               group="system">
+        <![CDATA[
+          this._mayReverse = true;
+          this._moveByOffsetFromUserEvent(this.scrollOnePage(-1), event);
+        ]]>
+      </handler>
       <handler event="keypress" keycode="VK_PAGE_DOWN" modifiers="control shift any"
-               action="this._moveByOffsetFromUserEvent(this.scrollOnePage(1), event);"
-               group="system"/>
+               group="system">
+        <![CDATA[
+          this._mayReverse = true;
+          this._moveByOffsetFromUserEvent(this.scrollOnePage(1), event);
+        ]]>
+      </handler>
       <handler event="keypress" key=" " modifiers="control" phase="target">
       <![CDATA[
         if (this.currentItem && this.selType == "multiple")
           this.toggleItemSelection(this.currentItem);
       ]]>
       </handler>
       <handler event="focus">
       <![CDATA[
--- a/toolkit/content/widgets/richlistbox.xml
+++ b/toolkit/content/widgets/richlistbox.xml
@@ -47,17 +47,17 @@
            extends="chrome://global/content/bindings/listbox.xml#listbox-base">
     <resources>
       <stylesheet src="chrome://global/skin/richlistbox.css"/>
     </resources>
 
     <content>
       <children includes="listheader"/>
       <xul:scrollbox allowevents="true" orient="vertical" anonid="main-box"
-                     flex="1" style="overflow: auto;">
+                     flex="1" style="overflow: auto;" xbl:inherits="dir,pack">
         <children/>
       </xul:scrollbox>
     </content>
 
     <implementation>
       <field name="_scrollbox">
         document.getAnonymousElementByAttribute(this, "anonid", "main-box");
       </field>
@@ -111,16 +111,63 @@
             this.dispatchEvent(event);
 
             // always call this (allows a commandupdater without controller)
             document.commandDispatcher.updateCommands("richlistbox-select");
           ]]>
         </body>
       </method>
 
+      <!-- We override base-listbox here because those methods don't take dir
+           into account on listbox (which doesn't support dir yet) -->
+      <method name="getNextItem">
+        <parameter name="aStartItem"/>
+        <parameter name="aDelta"/>
+        <body>
+        <![CDATA[
+          var prop = this.dir == "reverse" && this._mayReverse ?
+                                                "previousSibling" :
+                                                "nextSibling";
+          while (aStartItem) {
+            aStartItem = aStartItem[prop];
+            if (aStartItem && aStartItem instanceof
+                Components.interfaces.nsIDOMXULSelectControlItemElement &&
+                (!this._userSelecting || this._canUserSelect(aStartItem))) {
+              --aDelta;
+              if (aDelta == 0)
+                return aStartItem;
+            }
+          }
+          return null;
+        ]]></body>
+      </method>
+
+      <method name="getPreviousItem">
+        <parameter name="aStartItem"/>
+        <parameter name="aDelta"/>
+        <body>
+        <![CDATA[
+          var prop = this.dir == "reverse" && this._mayReverse ?
+                                                "previousSibling" :
+                                                "nextSibling";
+          while (aStartItem) {
+            aStartItem = aStartItem[prop];
+            if (aStartItem && aStartItem instanceof
+                Components.interfaces.nsIDOMXULSelectControlItemElement &&
+                (!this._userSelecting || this._canUserSelect(aStartItem))) {
+              --aDelta;
+              if (aDelta == 0)
+                return aStartItem;
+            }
+          }
+          return null;
+        ]]>
+        </body>
+      </method>
+
       <method name="appendItem">
         <parameter name="aLabel"/>
         <parameter name="aValue"/>
         <body>
           return this.insertItemAt(-1, aLabel, aValue);
         </body>
       </method>
 
@@ -298,19 +345,23 @@
         </body>
       </method>
 
     <!-- richlistbox specific -->
       <property name="children" readonly="true">
         <getter>
           <![CDATA[
             var childNodes = [];
-            for (var child = this.firstChild; child; child = child.nextSibling) {
+            var isReverse = this.dir == "reverse" && this._mayReverse;
+            var child = isReverse ? this.lastChild : this.firstChild;
+            var prop = isReverse ? "previousSibling" : "nextSibling";
+            while (child) {
               if (child instanceof Components.interfaces.nsIDOMXULSelectControlItemElement)
                 childNodes.push(child);
+              child = child[prop];
             }
             return childNodes;
           ]]>
         </getter>
       </property>
 
       <field name="_builderListener" readonly="true">
         <![CDATA[
@@ -350,20 +401,26 @@
               if (!currentItem && this._currentIndex)
                 currentItem = this.getItemAtIndex(Math.min(
                   this._currentIndex - 1, this.getRowCount()));
               if (currentItem) {
                 this.currentItem = currentItem;
                 if (this.selType != "multiple" && this.selectedCount == 0)
                   this.selectedItem = currentItem;
 
-                if (this.scrollBoxObject.height)
+                if (this.scrollBoxObject.height) {
                   this.ensureElementIsVisible(currentItem);
-                else // XXX hack around a bug in ensureElementIsVisible
-                  this.ensureElementIsVisible(currentItem.previousSibling);
+                }
+                else {
+                  // XXX hack around a bug in ensureElementIsVisible as it will
+                  // scroll beyond the last element, bug 493645.
+                  var previousElement = this.dir == "reverse" ? currentItem.nextSibling :
+                                                                currentItem.previousSibling;
+                  this.ensureElementIsVisible(previousElement);
+                }
               }
               this._suppressOnSelect = suppressSelect;
               // XXX actually it's just a refresh, but at least
               // the Extensions manager expects this:
               this._fireOnSelect();
               return;
             }