Bug 785635 - Markup panel undo is broken. r=jwalker

browser/devtools/shared/Undo.jsm

@@ -19,14 +19,15 @@ var EXPORTED_SYMBOLS=["UndoStack"];
  * @param integer aMaxUndo Maximum number of undo steps.
  *   defaults to 50.
  */
-function UndoStack(aChange, aMaxUndo)
+function UndoStack(aMaxUndo)
 {
   this.maxUndo = aMaxUndo || 50;
   this._stack = [];
 }
 
 UndoStack.prototype = {
-  // Current index into the undo .
+  // Current index into the undo stack.  Is positioned after the last
+  // currently-applied change.
   _index: 0,
 
   // The current batch depth (see startBatch() for details)
@@ -64,8 +65,9 @@ UndoStack.prototype = {
       return;
     }
 
-    // Cut off the undo stack wherever we currently are.
-    let start = Math.max(++this._index - this.maxUndo, 0);
+    // Cut off the end of the undo stack at the current index,
+    // and the beginning to prevent a stack larger than maxUndo.
+    let start = Math.max((this._index + 1) - this.maxUndo, 0);
     this._stack = this._stack.slice(start, this._index);
 
     let batch = this._batch;
@@ -83,6 +85,7 @@ UndoStack.prototype = {
       }
     };
     this._stack.push(entry);
+    this._index = this._stack.length;
     entry.do();
     this._change();
   },
@@ -127,7 +130,7 @@ UndoStack.prototype = {
    */
   canRedo: function Undo_canRedo()
   {
-    return this._stack.length >= this._index;
+    return this._stack.length > this._index;
   },
 
   /**

browser/devtools/shared/test/Makefile.in

@@ -11,6 +11,8 @@ relativesrcdir  = @relativesrcdir@
 
 include $(DEPTH)/config/autoconf.mk
 
+XPCSHELL_TESTS = unit
+
 MOCHITEST_BROWSER_FILES = \
   browser_browser_basic.js \
   browser_promise_basic.js \

browser/devtools/shared/test/unit/test_undoStack.js

@@ -0,0 +1,87 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const Cu = Components.utils;
+Cu.import("resource:///modules/devtools/Undo.jsm")
+
+const MAX_SIZE = 5;
+
+function run_test()
+{
+  let str = "";
+  let stack = new UndoStack(MAX_SIZE);
+
+  function add(ch) {
+    stack.do(function() {
+      str += ch;
+    }, function() {
+      str = str.slice(0, -1);
+    });
+  }
+
+  do_check_false(stack.canUndo());
+  do_check_false(stack.canRedo());
+
+  // Check adding up to the limit of the size
+  add("a");
+  do_check_true(stack.canUndo());
+  do_check_false(stack.canRedo());
+
+  add("b");
+  add("c");
+  add("d");
+  add("e");
+
+  do_check_eq(str, "abcde");
+
+  // Check a simple undo+redo
+  stack.undo();
+
+  do_check_eq(str, "abcd");
+  do_check_true(stack.canRedo());
+
+  stack.redo();
+  do_check_eq(str, "abcde")
+  do_check_false(stack.canRedo());
+
+  // Check an undo followed by a new action
+  stack.undo();
+  do_check_eq(str, "abcd");
+
+  add("q");
+  do_check_eq(str, "abcdq");
+  do_check_false(stack.canRedo());
+
+  stack.undo();
+  do_check_eq(str, "abcd");
+  stack.redo();
+  do_check_eq(str, "abcdq");
+
+  // Revert back to the beginning of the queue...
+  while (stack.canUndo()) {
+    stack.undo();
+  }
+  do_check_eq(str, "");
+
+  // Now put it all back....
+  while (stack.canRedo()) {
+    stack.redo();
+  }
+  do_check_eq(str, "abcdq");
+
+  // Now go over the undo limit...
+  add("1");
+  add("2");
+  add("3");
+
+  do_check_eq(str, "abcdq123");
+
+  // And now undoing the whole stack should only undo 5 actions.
+  while (stack.canUndo()) {
+    stack.undo();
+  }
+
+  do_check_eq(str, "abc");
+}

browser/devtools/shared/test/unit/xpcshell.ini

@@ -0,0 +1,5 @@
+[DEFAULT]
+head =
+tail =
+
+[test_undoStack.js]

testing/xpcshell/xpcshell.ini

@@ -91,6 +91,7 @@ skip-if = os == "android"
 [include:browser/components/places/tests/unit/xpcshell.ini]
 [include:browser/components/privatebrowsing/test/unit/xpcshell.ini]
 [include:browser/components/shell/test/unit/xpcshell.ini]
+[include:browser/devtools/shared/test/unit/xpcshell.ini]
 [include:extensions/spellcheck/hunspell/tests/unit/xpcshell.ini]
 [include:toolkit/components/search/tests/xpcshell/xpcshell.ini]
 [include:toolkit/mozapps/shared/test/unit/xpcshell.ini]