Bug 473412 - Editable tree cells cannot be activated by a keystroke. r=enn, sr=robstrong

toolkit/content/tests/widgets/tree_shared.js

@@ -31,7 +31,6 @@ function testtag_tree(treeid, treerowinfoid, seltype, columnstype, testid)
   document.addEventListener("keypress", preventDefault, false);
 
   var multiple = (seltype == "multiple");
-  var editable = false;
 
   var tree = document.getElementById(treeid);
   var treerowinfo = document.getElementById(treerowinfoid);
@@ -60,7 +59,7 @@ function testtag_tree(treeid, treerowinfoid, seltype, columnstype, testid)
 
   testtag_tree_TreeView(tree, testid, rowInfo);
 
-  is(tree.editable, editable, "editable");
+  is(tree.editable, false, "tree should not be editable");
   // currently, the editable flag means that tree editing cannot be invoked
   // by the user. However, editing can still be started with a script.
   is(tree.editingRow, -1, testid + " initial editingRow");
@@ -68,8 +67,20 @@ function testtag_tree(treeid, treerowinfoid, seltype, columnstype, testid)
 
   testtag_tree_UI_editing(tree, testid, rowInfo);
 
+  is(tree.editable, false, "tree should not be editable after testtag_tree_UI_editing");
+  // currently, the editable flag means that tree editing cannot be invoked
+  // by the user. However, editing can still be started with a script.
+  is(tree.editingRow, -1, testid + " initial editingRow (continued)");
+  is(tree.editingColumn, null, testid + " initial editingColumn (continued)");
+
   var ecolumn = tree.columns[0];
-  tree.startEditing(1, ecolumn);
+  ok(!tree.startEditing(1, ecolumn), "non-editable trees shouldn't start editing");
+  is(tree.editingRow, -1, testid + " failed startEditing shouldn't set editingRow");
+  is(tree.editingColumn, null, testid + " failed startEditing shouldn't set editingColumn");  
+  
+  tree.editable = true;
+
+  ok(tree.startEditing(1, ecolumn), "startEditing should have returned true");
   is(tree.editingRow, 1, testid + " startEditing editingRow");
   is(tree.editingColumn, ecolumn, testid + " startEditing editingColumn");
   is(tree.getAttribute("editing"), "true", testid + " startEditing editing attribute");
@@ -95,12 +106,14 @@ function testtag_tree(treeid, treerowinfoid, seltype, columnstype, testid)
   tree.stopEditing(true);
   is(tree.view.getCellText(1, ecolumn), "Changed Value", testid + "edit cell accept");
 
-  // this cell cannot be edited
+  // this cell can be edited, but stopEditing(false) means don't accept the change.
   tree.startEditing(1, ecolumn);
   inputField.value = "Second Value";
   tree.stopEditing(false);
   is(tree.view.getCellText(1, ecolumn), "Changed Value", testid + "edit cell no accept");
 
+  tree.editable = false;
+
   // do the sorting tests last as it will cause the rows to rearrange
   // skip them for the custom tree view
   if (testid !="tree view")
@@ -608,6 +621,24 @@ function testtag_tree_UI_editing(tree, testid, rowInfo)
   if (tree.view.isContainer(row))
     wasOpen = tree.view.isContainerOpen(row);
 
+  // Test whether a keystroke can enter text entry, and another can exit.
+  if (tree.selType == "cell")
+  {
+    tree.stopEditing(false);
+    ok(!tree.editingColumn, "Should not be editing tree cell now");
+    tree.view.selection.currentColumn = ecolumn;
+    tree.currentIndex = rowIndex;
+
+    const isMac = (navigator.platform.indexOf("Mac") >= 0);
+    const StartEditingKey = isMac ? "VK_ENTER" : "VK_F2";
+    synthesizeKey(StartEditingKey, {});
+    is(tree.editingColumn, ecolumn, "Should be editing tree cell now");
+    synthesizeKey("VK_ESCAPE", {});
+    ok(!tree.editingColumn, "Should not be editing tree cell now");
+    is(tree.currentIndex, rowIndex, "Current index should not have changed");
+    is(tree.view.selection.currentColumn, ecolumn, "Current column should not have changed");
+  }
+
   mouseDblClickOnCell(tree, rowIndex, ecolumn, testid + "edit on double click");
   is(tree.editingColumn, ecolumn, testid + "editing column");
   is(tree.editingRow, rowIndex, testid + "editing row");
@@ -642,23 +673,8 @@ function testtag_tree_UI_editing(tree, testid, rowInfo)
   // tree.stopEditing(true);
   // is(tree.view.getCellText(0, ecolumn), "b", testid + "edit cell");
 
-  if (1) // XXXndeakin disable these tests for now
-    return;
-
-  tree.startEditing(0, ecolumn);
-  inputField.value = "Value for Return";
-  synthesizeKey("VK_RETURN", {});
-  is(tree.view.getCellText(0, ecolumn), "Value for Return", testid + "edit cell return");
-
-  tree.startEditing(0, ecolumn);
-  inputField.value = "Value for Enter";
-  synthesizeKey("VK_ENTER", {});
-  is(tree.view.getCellText(0, ecolumn), "Value for Enter", testid + "edit cell enter");
-
-  tree.startEditing(0, ecolumn);
-  inputField.value = "Value for Escape";
-  synthesizeKey("VK_ESCAPE", {});
-  is(tree.view.getCellText(0, ecolumn), "Value for Enter", testid + "edit cell escape");
+  // Restore initial state.
+  tree.stopEditing(false);
 }
 
 function testtag_tree_TreeSelection_UI_cell(tree, testid, rowInfo)

toolkit/content/widgets/tree.xml

@@ -340,12 +340,15 @@
         <parameter name="column"/>
         <body>
           <![CDATA[
+            if (!this.editable)
+              return false;
             if (row < 0 || row >= this.view.rowCount || !column)
-              return;
+              return false;
             if (column.type != Components.interfaces.nsITreeColumn.TYPE_TEXT ||
                 column.cycler || !this.view.isEditable(row, column))
-              return;
+              return false;
 
+            // Beyond this point, we are going to edit the cell.
             if (this._editingColumn)
               this.stopEditing();
 
@@ -398,6 +401,7 @@
             this._editingColumn = column;
 
             this.setAttribute("editing", "true");
+            return true;
           ]]>
         </body>
       </method>
@@ -656,6 +660,28 @@
           ]]>
         </body>
       </method>
+      <method name="_handleEnter">
+        <parameter name="event"/>
+        <body><![CDATA[
+          if (this._editingColumn) {
+            this.stopEditing(true);
+            this.focus();
+            return true;
+          }
+
+#ifdef XP_MACOSX
+          // See if we can edit the cell.
+          var row = this.currentIndex;
+          if (this._cellSelType) {
+            var column = this.view.selection.currentColumn;
+            var startedEditing = this.startEditing(row, column);
+            if (startedEditing)
+              return true;
+          }
+#endif
+          return this.changeOpenState(this.currentIndex);
+        ]]></body>
+      </method>
     </implementation>
     
     <handlers>
@@ -709,33 +735,30 @@
       <handler event="blur" phase="capturing"
                action="if (event.originalTarget == this.inputField.inputField) this.stopEditing(true);"/>
       <handler event="keypress" keycode="VK_ENTER">
-        <![CDATA[
-          if (this._editingColumn) {
-            this.stopEditing(true);
-            this.focus();
-          }
-          else {
-            if (!this.changeOpenState(this.currentIndex))
-              return; // don't consume the event if the open state wasn't changed
-          }
+        if (this._handleEnter(event)) {
           event.stopPropagation();
           event.preventDefault();
-        ]]>
+        }
       </handler>
       <handler event="keypress" keycode="VK_RETURN">
-        <![CDATA[
-          if (this._editingColumn) {
-            this.stopEditing(true);
-            this.focus();
-          }
-          else {
-            if (!this.changeOpenState(this.currentIndex))
-              return; // don't consume the event if the open state wasn't changed
-          }
+        if (this._handleEnter(event)) {
           event.stopPropagation();
           event.preventDefault();
+        }
+      </handler>
+#ifndef XP_MACOSX
+      <!-- Use F2 key to enter text editing. -->
+      <handler event="keypress" keycode="VK_F2">
+        <![CDATA[
+          if (!this._cellSelType)
+            return;
+          var row = this.currentIndex;
+          var column = this.view.selection.currentColumn;
+          this.startEditing(row, column);
         ]]>
       </handler>
+#endif // XP_MACOSX
+
       <handler event="keypress" keycode="VK_ESCAPE">
         <![CDATA[
           if (this._editingColumn) {
@@ -1130,7 +1153,7 @@
         var obj = {};
         tbo.getCellAt(event.clientX, event.clientY, {}, col, obj);
 
-        if (this.parentNode.editable && obj.value != "twisty")
+        if (obj.value != "twisty")
           this.parentNode.startEditing(row, col.value);
 
         if (this.parentNode._editingColumn || !tbo.view.isContainer(row))